tbnobody / OpenDTU

Software for ESP32 to talk to Hoymiles/TSUN/Solenso Inverters
GNU General Public License v2.0
1.8k stars 499 forks source link

> you mean something like this? #2346

Closed mathieucarbou closed 2 hours ago

mathieucarbou commented 2 hours ago

you mean something like this?

oh nooooooo 😭

This one is crazy.

The fix I did (and reverted recently) which seems to work for most people, makes no sense in the code : I have looked at many many forks having the equivalent code and none of then have such fix applied. But if we apply it, this issue disappears for most people, except some (someone was able to still get it with the new fix - https://github.com/mathieucarbou/AsyncTCP/issues/23#issuecomment-2388809158).

With recent changes with middleware introduction, the little code I got to verify and reproduce the bug in a reliable way does not allow me to reproduce anymore, because the only way to reproduce was to insert some sort of delay between the commit of the response on the network and the end of the callback processing the request handler. Typically, in a request handler, doing something like this:

  server.on("/issue-14", HTTP_GET, [](AsyncWebServerRequest* request) {
    digitalWrite(4, HIGH);
    request->send(LittleFS, "/index.txt", "text/pain");
    delay(500);
    digitalWrite(4, LOW);
  });

Before, request->send() would write onto the network, and the delay after would cause the crash (always reproductible).

With middleware, request->send() does nothing on the network. So there is just a delay applied. The lib will commit the response at the end of the middleware chain execution. So there is no user code that could add a delay after the response is really sent on the network now.

What would be interesting for me is to have something to reproduce it.

Also, otherwise, when it happens, I would like to know if it was during the execution of a request handler which is doing a lot of operation, eventually I/O ops or eventually with delay() or yield() calls, which could make the CPU swap to another task at some point.

That's all hypothesis for now.

Putting back the fix sadly even if it works for some, it won't for some others (like the issue references above - the guy tested on 2 different boards)

Originally posted by @mathieucarbou in https://github.com/tbnobody/OpenDTU/discussions/2326#discussioncomment-10923810

mathieucarbou commented 2 hours ago

mistake!