Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebServer times out when loading NoVNC #8

Closed
i-am-shodan opened this issue Oct 1, 2024 · 13 comments
Closed

WebServer times out when loading NoVNC #8

i-am-shodan opened this issue Oct 1, 2024 · 13 comments

Comments

@i-am-shodan
Copy link
Owner

USBArmyKnife uses a fork of the ESPAsyncWebServer. Whilst this has got a lot better under its new maintainer I've noticed a few issues where the webserver recieves a high volume of requests. This is especially clear when Chrome/Edge tries to load a ton of the files needed for NoVNC.

mathieucarbou/ESPAsyncWebServer#70 maybe related

@i-am-shodan i-am-shodan changed the title WebServer has stability issues, especially when loading NoVNC WebServer has when loading NoVNC Oct 1, 2024
@i-am-shodan i-am-shodan changed the title WebServer has when loading NoVNC WebServer times out when loading NoVNC Oct 3, 2024
@mathieucarbou
Copy link

FYI:

I've just fixed a bug regarding string usages for ESP8266 (long time bug):

https://github.com/mathieucarbou/ESPAsyncWebServer/releases/tag/v3.3.16

If possible, please let me know if it solves the issue....

Regarding the slowdown, there were an issue in AsyncTCP lib for ESP32 which has been fixed also. For ESP32, you can use either AsyncTCP or AsyncTCPSock:

@i-am-shodan
Copy link
Owner Author

Interesting, have you got any more details on why constexp was bad. I'm using constexp in places.

@mathieucarbou
Copy link

mathieucarbou commented Oct 15, 2024

Interesting, have you got any more details on why constexp was bad. I'm using constexp in places.

its not bad. It's just that for ESP8266, strings have to be put in flash with PROGMEM to not count against ram memory.

And the define was wrong (ESP8622 instead of ESP8266), which lead all the strings to be kept in ram instead of flash

mathieucarbou/ESPAsyncWebServer@65906f7#diff-eff42aa28535f8fe20945bef89f9d8640da70cf6fe881c51b9af0b951b4b36c2R185

@mathieucarbou
Copy link

Oh boy! I didn't know your projects! That's cool: I didn't know the esp-idf api had a promiscuous mode!

@i-am-shodan
Copy link
Owner Author

@mathieucarbou I tried your changes and what I can say is that every release things seems to get a lot better. Sadly not well enough to resolve this issue for me.

In my case I'm using AsyncTCP and ESPAsyncWebServer straight from your git repo (thanks for all the work maintaining all of this by the way).

I'm still getting issues trying to load lots of files with varying sizes. Hitting F5 in the browser to try and get these to be returned often results in the ESPAsyncWebServer deadlocking - the device itself doesn't reset. Here is an example of what Edge sees:

image

Everything is going good and then one file fails to load :(

The files i'm trying to load are here:
https://github.com/i-am-shodan/USBArmyKnife/tree/master/ui/vnc
These are converted by a build script into gzipped compressed versions to save on RAM and transfer. Each page is defined as:
const uint8_t PROGMEM bootstrapMinCssGz as an example

I then autogenerate a really big table of URL -> PROGMEM data pointer. When a request comes through this is looked up and sent across with:

AsyncWebServerResponse *response = request->beginResponse_P(200, mime != nullptr ? mime : unknown, data.first, data.second, nullptr);

I wondering if you've got any ideas. Also no idea what AsyncTCPSock is and whether it could help here. Which is better?

@mathieucarbou
Copy link

Don't use _P methods on Esp32: they are deprecated

Also, have a look at the the ESPAsyncWebServer project page (my fork) there are some perf test and also some explications to swap AsyncTCP by AsyncTCPSock which is a better implementation

AsyncTCPSock was written by the same guy who also maintained ESPAsyncWebServer fork in the past and fixed many issues. My ESPAsyncWebServer fork is based on his fork and his AsyncTCPSock lib is based on bsd sockets and fixes the limitations over concurrent requests handling (16 max) for AsyncTCP and fixes it's design based on a slower queue system.

@i-am-shodan
Copy link
Owner Author

So I had a go migrating to AsyncTCPSock. Firstly it looks like it doesn't compile under the latest Arduino library. In AsyncTcp.h I had to change the code to this to get it to compile.

int res = getsockopt(_socket, IPPROTO_TCP, TCP_NODELAY, (char *)&flag, (socklen_t*) &size);

I ran the code and anecdotally it seemed worse that AsyncTCP. Not sure why exactly, just more failed requests.

@mathieucarbou
Copy link

Look here: https://github.com/mathieucarbou/ESPAsyncWebServer?tab=readme-ov-file#dependencies

It compiles, its tested and perf tested on Arduino 3 and latest rc1 also.

@i-am-shodan
Copy link
Owner Author

Using the code from the dev branch worked a treat. Default branch seemed to be the problem for me. However my testing sadly still stands and I don't see any improvement with AsyncTCPSock.

@mathieucarbou
Copy link

mathieucarbou commented Oct 16, 2024

Do you have a waterfall view of your requests ? I know AsyncTCP has a concurrency limit (lwip layer) of 16 slots and cannot handle more than 16 clients at a time. AsyncTCPSock does better.

But what could happen is that your browser is querying too many resources at once, forcing the ESP to use more memory to serve them.

If you are forced to split into several files and cannot regroup in 1 minified bigger file, like all other ESP32 projects are doing usually, then, an option could be to use a JS library that will load these scripts one by one : did you try that ?

In any case, if you want the cause of this issue to be found, you need more data, like maybe print the free heap before serving fa file, print the pointers and size to be sure and their content in the console, etc.

There are too many hypothesis right now to troubleshoot anything.

@i-am-shodan
Copy link
Owner Author

I think one large file would probably solve the problem here. Because NoVNC is a library from elsewhere merging everything together isn't going to be trivial. I didn't consider a library to load them one by one, I'll check that out as it might be a good compromise.

@mathieucarbou
Copy link

I remember we were doing that with https://requirejs.org 10 years ago.
But considering the recent evolution of JS, I bet it can be done in a few lines of code now...

@i-am-shodan
Copy link
Owner Author

I think i've fixed this by preloading all the resources one by one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants