lasselukkari / aWOT

Arduino web server library.
MIT License
283 stars 41 forks source link

Awot crash on slow network with esp8266 + ethernet #130

Closed fuxionlab closed 2 years ago

fuxionlab commented 3 years ago

We are using ethernet on esp8266 deployed on congested network, we often see the esp8266 crashing and upon debugging it points to awot while statements of the library, so we decided adding yield on every while statements with the client.available() and it fixed the frequent crash. We have realized the solution since slow network could make the response wait for too long and since using a while(client.available) without yield inside would potentially trigger wdt resets.

lasselukkari commented 3 years ago

I have never considered this before. When used with the esp8266 WiFi the yield would be unnecessary because calling the client functions is enough keep the wifi stack alive, but with ethernet it makes sense that it behaves like this. It should be fairly easy to simulate the condition with a custom tcp client that just stops sending the data in the middle of the request. I'll try to get it fixed. Is is possible to seen the changes you have now made? What is your exact hardware setup?

lasselukkari commented 3 years ago

Do you mean that it happens when writing when you talk about the response waiting?

fuxionlab commented 3 years ago

Sorry for late response, below are some of the lines, theres more of this while with the yield() added, we have rushed adding those yield as a quick fix. Hardware is esp8266 nodemcu in particular and a w5500 ethernet ` bool Request::m_expect(const char expected) { const char candidate = expected; while (*candidate != 0) { yield(); int ch = m_timedRead(); if (ch == -1) { return false; }

if (tolower(ch) != tolower(*candidate++)) {
  push(ch);

  while (--candidate != expected) {
    yield();
    push(candidate[-1]);
  }

  return false;
}

}

return true; }

bool Request::m_skipSpace() { int ch;

while ((ch = m_timedRead()) != -1 && (ch == ' ' || ch == '\t')){ yield(); }

if (ch == -1) { return false; }

push(ch);

return true; }

void Request::m_reset() { HeaderNode *headerNode = m_headerTail; while (headerNode != NULL) { yield(); headerNode->buffer[0] = '\0'; headerNode = headerNode->next; } } `

lasselukkari commented 3 years ago

Ok. I think it should be enough to yield when any read or write function is called in a loop. It should no be required when looping over the data that is already in the memory. I can try to prepare you a branch that you can try out. I'm in the middle of moving to a new home so this may take while.

lasselukkari commented 3 years ago

What ethernet library are you using? To be honest I'm not completely sure should this problem be actually handled there.

fuxionlab commented 3 years ago

We are using this library https://github.com/adafruit/Ethernet2/tree/master/examples, we have no problems so far with the above modifications, we've also increased spi speed by tweaking the ethernet2 library since esp8266 can support higher spi speeds for the ethernet. Thank you for your time looking into this issue for someone with a rare setup like us (esp + ethernet). Everything works perfectly with esp8266 builtin wifi which the majority would prefer.

lasselukkari commented 3 years ago

The original author of Ethernet library seems to think the calls to yield should be in the library. See his comment here: https://github.com/esp8266/Arduino/issues/5622#issuecomment-454874559. I also think this issue is more generic and because of that it should be fixed at the root. This will cause problems in many places that expect it to work differently.