Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

textAll crash after ERROR: Too many messages queued #1363

Open
ninja- opened this issue Nov 20, 2023 · 2 comments
Open

textAll crash after ERROR: Too many messages queued #1363

ninja- opened this issue Nov 20, 2023 · 2 comments

Comments

@ninja-
Copy link

ninja- commented Nov 20, 2023

void AsyncWebSocket::textAll(const char * message, size_t len){
  AsyncWebSocketMessageBuffer * WSBuffer = makeBuffer((uint8_t *)message, len);
    textAll(WSBuffer);
}

void AsyncWebSocket::textAll(AsyncWebSocketMessageBuffer * buffer){
  if (!buffer) return;
  buffer->lock();
  for(const auto& c: _clients){
    if(c->status() == WS_CONNECTED){
        c->text(buffer);
    }
  }
  buffer->unlock();
  _cleanBuffers();
}

void AsyncWebSocketClient::text(AsyncWebSocketMessageBuffer * buffer)
{
  _queueMessage(new AsyncWebSocketMultiMessage(buffer));
}

void AsyncWebSocketClient::_queueMessage(AsyncWebSocketMessage *dataMessage){
  if(dataMessage == NULL)
    return;
  if(_status != WS_CONNECTED){
    delete dataMessage;
    return;
  }
  if(_messageQueue.length() >= WS_MAX_QUEUED_MESSAGES){
      ets_printf("ERROR: Too many messages queued\n");
      delete dataMessage;
  } else {
      _messageQueue.add(dataMessage);
  }
  if(_client->canSend())
    _runQueue();
}

delete dataMessage is called multiple times, causing memory corruption

@ninja-
Copy link
Author

ninja- commented Nov 20, 2023

ah, I think my diagnosis was not correct, because AsyncWebSocketMultiMessage is created per-connection and deleted, and not the buffer itself. the crash under > 8 clients is coming from some other bug...

@ninja-
Copy link
Author

ninja- commented Nov 20, 2023

I guess this will be fixed after manually applying this set of patches Aircoookie/WLED#3382 (comment), which I will try soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant