Skip to content

Commit 49de180

Browse files
committed
Set CloseClientOnQueueFull to false by default
This PR changes the default flag for CloseClientOnQueueFull to set it to false. false should be the sensible default for this library especially after teh recent refatcoring allowing for a very fast message delivery. Applications have to check the status when sending and queue status to check if they need to decrease the sending rate or close the client. Ref: #433
1 parent a9fedfe commit 49de180

2 files changed

Lines changed: 27 additions & 16 deletions

File tree

src/AsyncWebSocket.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ bool AsyncWebSocketClient::_queueMessage(AsyncWebSocketSharedBuffer buffer, uint
467467
}
468468

469469
if (_messageQueue.size() >= WS_MAX_QUEUED_MESSAGES) {
470-
if (closeWhenFull) {
470+
if (_closeWhenFull) {
471471
_status = WS_DISCONNECTED;
472472

473473
async_ws_log_w("[%s][%" PRIu32 "] Too many messages queued: closing connection", _server->url(), _clientId);

src/AsyncWebSocket.h

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ class AsyncWebSocketClient {
225225
mutable asyncsrv::mutex_type _queue_lock;
226226
std::deque<AsyncWebSocketControl> _controlQueue;
227227
std::deque<AsyncWebSocketMessage> _messageQueue;
228-
bool closeWhenFull = true;
228+
bool _closeWhenFull = false;
229229

230230
AwsFrameInfo _pinfo;
231231

@@ -274,29 +274,40 @@ class AsyncWebSocketClient {
274274
return _pinfo;
275275
}
276276

277-
// - If "true" (default), the connection will be closed if the message queue is full.
277+
// CloseClientOnQueueFull:
278+
//
279+
// - If "true", the client will be closed if the message queue becomes full.
278280
// This is the default behavior in yubox-node-org, which is not silently discarding messages but instead closes the connection.
279281
// 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,
280-
// and so on, causing a resource exhaustion.
282+
// and so on, causing a resource exhaustion.
283+
// Also this can lead to a crash as explained in this issue: https://github.com/ESP32Async/ESPAsyncWebServer/issues/433
281284
//
282-
// - If "false", the incoming message will be discarded if the queue is full.
285+
// - If "false" (default in this library), the incoming message will be discarded if the queue is full.
283286
// This is the default behavior in the original ESPAsyncWebServer library from me-no-dev.
284287
// This behavior allows the best performance at the expense of unreliable message delivery in case the queue is full (some messages may be lost).
285288
//
286-
// - In any case, when the queue is full, a message is logged.
287-
// - IT is recommended to use the methods queueIsFull(), availableForWriteAll(), availableForWrite(clientId) to check if the queue is full before sending a message.
289+
// 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:
290+
// - 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
291+
// - 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.
292+
//
293+
// In case your application requires a fast and high frequency message sending and you forsee some queue usage, you can:
294+
// - increase the queue side to allow some room
295+
// - 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.
296+
//
297+
// This has to be an application-specific deicison that the library cannot take for you.
298+
// Here are a list of some functions that you can use and check the boolean value returned:
299+
// - the send methods
300+
// - queueIsFull()
301+
// - availableForWriteAll()
302+
// - availableForWrite(clientId)
288303
//
289-
// Usage:
290-
// - can be set in the onEvent listener when connecting (event type is: WS_EVT_CONNECT)
304+
// When the queue is full, a message is logged in case it is discarded.
291305
//
292-
// Use cases:,
293-
// - if using websocket to send logging messages, maybe some loss is acceptable.
294-
// - But if using websocket to send UI update messages, maybe the connection should be closed and the UI redrawn.
295306
void setCloseClientOnQueueFull(bool close) {
296-
closeWhenFull = close;
307+
_closeWhenFull = close;
297308
}
298309
bool willCloseClientOnQueueFull() const {
299-
return closeWhenFull;
310+
return _closeWhenFull;
300311
}
301312

302313
IPAddress remoteIP() const;
@@ -319,8 +330,8 @@ class AsyncWebSocketClient {
319330
}
320331

321332
// data packets
322-
void message(AsyncWebSocketSharedBuffer buffer, uint8_t opcode = WS_TEXT, bool mask = false) {
323-
_queueMessage(buffer, opcode, mask);
333+
bool message(AsyncWebSocketSharedBuffer buffer, uint8_t opcode = WS_TEXT, bool mask = false) {
334+
return _queueMessage(buffer, opcode, mask);
324335
}
325336
bool queueIsFull() const;
326337
size_t queueLen() const;

0 commit comments

Comments
 (0)