ESPAsyncWebServer icon indicating copy to clipboard operation
ESPAsyncWebServer copied to clipboard

textAll crash after ERROR: Too many messages queued

Open ninja- opened this issue 2 years ago • 2 comments

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- avatar Nov 20 '23 21:11 ninja-

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- avatar Nov 20 '23 21:11 ninja-

I guess this will be fixed after manually applying this set of patches https://github.com/Aircoookie/WLED/issues/3382#issuecomment-1751363805, which I will try soon

ninja- avatar Nov 20 '23 21:11 ninja-