Merge pull request #434 from ESP32Async/issue/433

Set CloseClientOnQueueFull to false by default

Author: mathieucarbou

docs/websockets.md

@@ -171,6 +171,38 @@ const uint8_t flash_binary[] PROGMEM = { 0x01, 0x02, 0x03, 0x04 };
 client->binary(flash_binary, 4);
 ```
 
+### Queue full behavior: `setCloseClientOnQueueFull()`
+
+When a client cannot keep up, outgoing WebSocket messages are queued.
+If the queue reaches `WS_MAX_QUEUED_MESSAGES`, new messages are either discarded or the client is closed, depending on this setting.
+
+```cpp
+client->setCloseClientOnQueueFull(bool close);
+```
+
+- `close == false` (default in this library): discard new messages when the queue is full.
+- `close == true`: close the client when the queue is full.
+
+We recommend using `false`.
+
+When the queue starts filling, prefer reducing your sending rate and/or explicitly closing the client according to your application policy.
+
+We do not recommend using `true` because it can lead to a crash under certain circumstances.
+
+You can combine this with:
+
+- `client->queueIsFull()`
+- `client->queueLen()`
+- `ws.availableForWrite(clientId)` and `ws.availableForWriteAll()`
+
+Typical usage is to set the policy when the client connects:
+
+```cpp
+if (type == WS_EVT_CONNECT) {
+  client->setCloseClientOnQueueFull(false); // default behavior
+}
+```
+
 ### Direct access to web socket message buffer
 
 When sending a web socket message using the above methods a buffer is created. Under certain circumstances you might want to manipulate or populate this buffer directly from your application, for example to prevent unnecessary duplications of the data. This example below shows how to create a buffer and print data to it from an ArduinoJson object then send it.

examples/arduino/WebSocket/WebSocket.ino

@@ -93,7 +93,6 @@ void setup() {
     if (type == WS_EVT_CONNECT) {
       ws.textAll("new client connected");
       Serial.println("ws connect");
-      client->setCloseClientOnQueueFull(false);
       client->ping();
 
     } else if (type == WS_EVT_DISCONNECT) {

examples/idf_component/websocket/main/main.cpp

@@ -37,7 +37,6 @@ void setup() {
     if (type == WS_EVT_CONNECT) {
       ws.textAll("new client connected");
       Serial.println("ws connect");
-      client->setCloseClientOnQueueFull(false);
       client->ping();
 
     } else if (type == WS_EVT_DISCONNECT) {

src/AsyncWebSocket.cpp

@@ -467,7 +467,7 @@ bool AsyncWebSocketClient::_queueMessage(AsyncWebSocketSharedBuffer buffer, uint
   }
 
   if (_messageQueue.size() >= WS_MAX_QUEUED_MESSAGES) {
-    if (closeWhenFull) {
+    if (_closeWhenFull) {
       _status = WS_DISCONNECTED;
 
       async_ws_log_w("[%s][%" PRIu32 "] Too many messages queued: closing connection", _server->url(), _clientId);

src/AsyncWebSocket.h

@@ -225,7 +225,7 @@ class AsyncWebSocketClient {
   mutable asyncsrv::mutex_type _queue_lock;
   std::deque<AsyncWebSocketControl> _controlQueue;
   std::deque<AsyncWebSocketMessage> _messageQueue;
-  bool closeWhenFull = true;
+  bool _closeWhenFull = false;
 
   AwsFrameInfo _pinfo;
 
@@ -274,29 +274,39 @@ class AsyncWebSocketClient {
     return _pinfo;
   }
 
-  //  - If "true" (default), the connection will be closed if the message queue is full.
+  // CloseClientOnQueueFull:
+  //
+  // - If "true", the client will be closed if the message queue becomes full.
   // This is the default behavior in yubox-node-org, which is not silently discarding messages but instead closes the connection.
   // The big issue with this behavior is  that is can cause the UI to automatically re-create a new WS connection, which can be filled again,
   // and so on, causing a resource exhaustion.
+  // Also this can lead to a crash as explained in this issue: https://github.com/ESP32Async/ESPAsyncWebServer/issues/433
   //
-  // - If "false", the incoming message will be discarded if the queue is full.
+  // - If "false" (default in this library), the incoming message will be discarded if the queue is full.
   // This is the default behavior in the original ESPAsyncWebServer library from me-no-dev.
   // This behavior allows the best performance at the expense of unreliable message delivery in case the queue is full (some messages may be lost).
   //
-  // - In any case, when the queue is full, a message is logged.
-  // - IT is recommended to use the methods queueIsFull(), availableForWriteAll(), availableForWrite(clientId) to check if the queue is full before sending a message.
+  // With recent refactorings of the library, the queue is barely used and the library supports a fast sending rate of messages. So if the queue is growing:
+  //  - either the server is sending messages at an insane fast rate, faster than what the client can acknowledge, which can be the case if the client is slow or if the messages are big and the network is slow,
+  //  - or there is a network issue causing the client to not receive messages, or network is broken. In that case, if the network is broken, the queue will fill temporarily until the connection is closed and client removed.
+  //
+  // In case your application requires a fast and high frequency message sending and you foresee some queue usage, you can:
+  //  - increase the queue side to allow some room
+  //  - check some functions status before or when sending in order to decrease your sending rate to let the queue drain, or take action by closing this client if necessary.
   //
-  // Usage:
-  //  - can be set in the onEvent listener when connecting (event type is: WS_EVT_CONNECT)
+  // This has to be an application-specific deicison that the library cannot take for you.
+  // Here are a list of some functions that you can use and check the boolean value returned:
+  // - the send methods
+  // - queueIsFull()
+  // - availableForWriteAll()
+  // - availableForWrite(clientId)
   //
-  // Use cases:,
-  // - if using websocket to send logging messages, maybe some loss is acceptable.
-  // - But if using websocket to send UI update messages, maybe the connection should be closed and the UI redrawn.
+  // When the queue is full, a message is logged in case it is discarded.
   void setCloseClientOnQueueFull(bool close) {
-    closeWhenFull = close;
+    _closeWhenFull = close;
   }
   bool willCloseClientOnQueueFull() const {
-    return closeWhenFull;
+    return _closeWhenFull;
   }
 
   IPAddress remoteIP() const;
@@ -319,8 +329,8 @@ class AsyncWebSocketClient {
   }
 
   // data packets
-  void message(AsyncWebSocketSharedBuffer buffer, uint8_t opcode = WS_TEXT, bool mask = false) {
-    _queueMessage(buffer, opcode, mask);
+  bool message(AsyncWebSocketSharedBuffer buffer, uint8_t opcode = WS_TEXT, bool mask = false) {
+    return _queueMessage(buffer, opcode, mask);
   }
   bool queueIsFull() const;
   size_t queueLen() const;