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

on 2.1.0 server.setNoDelay(true); generate exception(2) at start #1695

Closed
luc-github opened this issue Feb 28, 2016 · 6 comments
Closed

on 2.1.0 server.setNoDelay(true); generate exception(2) at start #1695

luc-github opened this issue Feb 28, 2016 · 6 comments

Comments

@luc-github
Copy link
Contributor

On 2.1.0 my project crash at start when wasn't using 2.0.0
I narrow down using the sample sketch WiFiTelnetToSerial.ino (https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266WiFi/examples/WiFiTelnetToSerial/WiFiTelnetToSerial.ino), which generate exception(2) at start.
commenting the line :

server.setNoDelay(true);

solve the issue.

This do not happen on 2.0.0.

2.1.0 sample sketch is compiled under windows 10 / IDE 1.6.5 and using nodemcu 1.0

@miky2k
Copy link

miky2k commented Mar 4, 2016

Confirmed for me.
there is possibility to back-port correction to 2.1.0 without to wait next version?

@tablatronix
Copy link
Contributor

yup same here latest master, WiFiServer.setNoDelay(true) causes wdt resets.

#include <ESP8266WiFi.h>
#include <Arduino.h>

WiFiServer TelnetServer(23);
WiFiClient Telnet;

void setup(){
  Serial.begin(115200);
  Serial.setDebugOutput(true);
  TelnetServer.begin();
//  TelnetServer.setNoDelay(true);
}

void loop(){
  if (TelnetServer.hasClient()){
    if (!Telnet || !Telnet.connected()){
      if(Telnet) Telnet.stop();
      Telnet = TelnetServer.available();
    } else {
      TelnetServer.available().stop();
    }
  }
  if (Telnet && Telnet.connected() && Telnet.available()){
    while(Telnet.available())
      Serial.write(Telnet.read());
  }
}

@igrr
Copy link
Member

igrr commented Mar 10, 2016

After a short investigation, setNoDelay on a WiFiServer instance had always resulted in undefined behaviour. But before we upgraded to 1.5.1 it didn't cause run-time issues (apparently).

For those who are interested, here is what's happening. We call tcp_nagle_disable on a server tcp_pcb structure. This macro expands into ((pcb)->flags |= TF_NODELAY). Flags is a member of tcp_pcb structure. The trouble is that tcp_listen function, which we call to start listening on a port, replaces tcp_pcb structure with a smaller tcp_pcb_listen structure.
https://github.com/kadamski/esp-lwip/blob/esp8266-1.4.1/src/core/tcp.c#L519-L570
https://github.com/kadamski/esp-lwip/blob/esp8266-1.4.1/src/include/lwip/tcp.h#L284-L294
Now tcp_pcb_listen structure doesn't have flags member! When we do tcp_nagle_disable, we write out of bounds of this structure.

So it seems that we have to remove setNoDelay function from WiFiServer class and update examples accordingly.

@luc-github
Copy link
Contributor Author

Thanks a lot @igrr - very clear
Additional question: if the flags TF_NODELAY is never used, should it be removed also ?
or do we expect in a future that tcp_pcb_listen function to support this flag ?

@igrr
Copy link
Member

igrr commented Mar 10, 2016

We will save no_delay flag in WiFiServer, and it will set this flag on each new client connection automatically. I'll post a commit soon. (i.e. will not remove setNoDelay function from WiFiServer).

@luc-github
Copy link
Contributor Author

Excellent - thanks a lot - you are the best as usual ^_^

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

No branches or pull requests

4 participants