WebSocket server does not limit connections per-se

[dronus]

The WebSocket server seem not to limit connections per-se, so on overuse becoming unsafe denial-of-service attacks with unlimited memory usage and thus crash potential.

For example, serving some UI using WebSockets, would allow users to open unlimited instances of the UI, using one websocket client each. On some point, methods like textAll() or similliar would crash, not able to allocate needed memory anymore.

So while there is the possibility to prune the connection list by calling ws.cleanupClients() "frequently", as proposed, there is no guarantee what is frequently enough to limit connections and thus used memory to any hard limit.

So the WebSocket server should prune connections on clients connecting and hard limit connections there. Otherwise there is no chance to get a guaranteed crash-save application - we cannot limit the browsers trying to connect.

[mathieucarbou]

The user is responsible to call cleanup in the loop and you an add a middleware to limit client count.

Look at the examples and doc.

[dronus]

Yes, but calling cleanup is no guarantee that any amount of connections are established between the calls to cleanup. And limit client count alone is also not guaranteeing anything, as single clients can queue up outgoing messages, with the total memory needed adding up by the size of message buffers still reachable by queued messages - this is quite intransparent and does not allow for predicting reasonable memory usage limits. So maybe there could be a global message buffer limit or something.

So currently strongly clamping down on the number of clients and max. message queue length is as close as we can get, but these limits needs to be very tight to prevent occasional peaks under bad circumstances, eg. several lagging clients each keeping up output message queue with larger buffers attached. Eg. an application that may server 10 clients under good circumstances need to be limited to 2 clients to prevent crash under worst case conditions. With the "10" and "2" both determined by experimentation without any guarantee it would not crash tomorrow.

[mathieucarbou]

Yes, but calling cleanup is no guarantee that any amount of connections are established between the calls to cleanup.

I agree, this is just a cleanup procedure user has to call

Like I said, there is already an example in the project on how to limit the number of concurrent WS clients by using middleware.

[willmmiles]

It's also worth noting that the underlying LwIP TCP stack has a hard cap on the number of active connections set by LWIP_MAX_SOCKETS, commonly set to 10 or 16. Additional connection requests will be rejected by the TCP stack and will never even reach AsyncWebServer.

[gnalbandian]

Maybe unrelated with this specific case, but since almost every crash related to the use of this library has something to do with memory allocation, why not use new with disabled exceptions (nothrow) and test if the allocation was successful?

[dronus]

@gnalbandian memory allocations may also occur elsewhere in application code, so exhausting resources may randomly deny allocations external to ESPAsyncWebserver code. So basically every other piece of code would have to be aware of the web server exhausting resources.

[gnalbandian]

Sure, but at least the code wouldn't randomly crash and it would be far easier to understand where or what's causing the problems.

[dronus]

No, it will still randomly crash - just not in the ESPAsyncWebserver code, but in other user code even if that is handling memory nicely! So it would be even worse - some totally sane code would crash, but the cause would be the webserver exhausting the memory - which makes debugging even harder!

[dronus]

Maybe the webserver could use its own allocator, based on a predefined static buffer. This way it may deny requests nicely by failing allocation, without compromising whole system stability. If too many clients connect, or too many responses are in-flight or whatever, a single or more connections would error out and close, but server and firmware will continue to run and accept connections in the future as the connection load drops.

[mathieucarbou]

There are examples of rate limit and concurrency limit middleware in the examples. I think the project already provides enough to the user to achieve that upfront of the request processing.

[dronus]

I agree with that, but as already mentioned there is no way to enforce an upper memory limit in this project. All of the limits discussed so far will reduce memory usage in some way, but without guarantees for any finite memory footprint. Which leaves the system susceptible for externally triggered crashes. That may be ok, if serving data is the main use case of the system. But it is not acceptable if the system is in realtime control of anything.

[dronus]

This is not a theoretical problem though, I currently working on a motion control project, and I already spent a lot of time reducing crashes from repeatable to occasionally, but all of them are triggered by network induced memory exhaustions. Even if I reduce websocket connections to a very low number, just three connected and talking clients may in adverse conditions, eg. slow or unreliable network, grow ESPAsyncWebservers memory demand until exhaustion.

[mathieucarbou]

Yes, there is always pros and cons in a solution. I think it is quite hard to have a finite memory model for a web server because it interacts with external components from Arduino Core or esp-idf which are not working with a finite memory model.

Did you try to configure the queue sizes to avoid enqueuing too many messages?