libesphttpd icon indicating copy to clipboard operation
libesphttpd copied to clipboard

Questions about concurrency and memory

Open simap opened this issue 5 years ago • 2 comments

Hi, and thank you for this fork, along with some of @jkent's cmake stuff I'm up and running!

I'm wondering about concurrency and access to the sendBuf.

I noticed some concurrency lock stuff around websocket broadcast, and it looks like the other sending functions don't have that, do they assume to only be called from inside a cgi handler?

Also poking around a bit I noticed the static send buffer changes. It can consume quite a lot of memory (maxConnections * HTTPD_MAX_SENDBUFF_LEN), but I like that it's not going to fragment memory.

If responses are assumed to be composed in callbacks only, is it possible for more than 1 sendBuf to be written to simultaneously from the server task? It doesn't look like there's an intermediate action that could cause writes to sendBuf that don't also get flushed.

It looks like every write to the sendBuf is protected by httpdPlatLock (even the async websocket broadcast). It looks like it is fully synchronous, and you won't have other tasks trying to mutate it simultaneously).

I'm thinking about trying a single static sendbuf per HttpdInstance.

Does this sound like it could work? Am I missing something?

simap avatar Oct 20 '19 17:10 simap

As a quick test/hack, I replaced all sendBuff and sendBuffLen references with a global. To make sure, I added a check to make sure when sendBuffLen is set to zero on the start that it is never nonzero, and it seems to work (ran a bunch of concurrent simultaneous downloads and the example test suite). Cuts per-connection memory down to 1120 (1024 of which comes from HTTPD_MAX_HEAD_LEN).

simap avatar Oct 20 '19 18:10 simap

@simap I've just checked your idea and tested a little with two browsers: one was loading OTA firmware and the second with configuration/websockets/cgi and everything seems to works fine. I going to use it for a while to verify everything if this is stable solution - thanks!

marbalon avatar Oct 28 '20 09:10 marbalon