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

sendBIN w/ headerToPayload seems unstable? (ESP8266) #350

Closed
simap opened this issue Jul 24, 2018 · 5 comments
Closed

sendBIN w/ headerToPayload seems unstable? (ESP8266) #350

simap opened this issue Jul 24, 2018 · 5 comments

Comments

@simap
Copy link
Contributor

simap commented Jul 24, 2018

When the system is under memory pressure, headerToPayload seems to break, sending frames that causes the browser to close the connection. e.g. in chrome many different errors can appear such as:

WebSocket connection to 'ws://192.168.1.223:81/' failed: Received start of new message but previous message is unfinished.

attaching a test setup in this gist including a program to run on an ESP8266 and an html page to drive it:
https://gist.github.com/simap/6723beab8668035d19b9d7f8aea07b2e

Clicking connect, then on the "Test w headerToPayload" button allocates a buffer with an extra WEBSOCKETS_MAX_HEADER_SIZE bytes, then writes offset by that, and calls sendBIN w/ headerToPayload true.

In the code, if fillSize is set to a small number reducing memory pressure, everything works OK.

This seems counterintuitive, since headerToPayload is supposed to reduce an extra malloc + memcpy in some cases.

It also breaks with some smaller payloads e.g.:

static const int blastSize = 256; 
@Links2004
Copy link
Owner

this can happen if the TCP stack can not handle to load.
how much free heap do you have when the problem starts?
from my experience the ESP gets instabile when you go below 6K free Heap.

skipping the copy step via headerToPayload will not stop the TCP stack from coping part of the data on the write call here:
https://github.com/Links2004/arduinoWebSockets/blob/master/src/WebSockets.cpp#L634
the write will end up here:
https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266WiFi/src/include/ClientContext.h#L330
https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266WiFi/src/include/ClientContext.h#L396
the clone of max 256*4 happens here:
https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266WiFi/src/include/ClientContext.h#L446
plus some additional stuff for Layer 2 + Layer 3.
printing and other String stuff will need some heap too.

can you put the two debug lines in that are here:
https://github.com/Links2004/arduinoWebSockets/blob/master/src/WebSockets.cpp#L640
https://github.com/Links2004/arduinoWebSockets/blob/master/src/WebSockets.cpp#L642
then we can see better what the TCP stack is doing.

@simap
Copy link
Contributor Author

simap commented Jul 26, 2018

The used heap in the test program is adjustable. When headerToPayload is enabled it is unstable w/ 8k, and stable with 20k. I haven't tested a lot to find the breaking point.

With as little as 4k of ram, I can reliably send long bursts of 256b packets UNLESS headerToPayload is enabled. and 256b of that is used to make the payload buffer.

Results of enabling those logging statements using the same settings as in the example gist:
https://gist.github.com/simap/591f166a8064e20c8fdc9bd871959b6e

@simap
Copy link
Contributor Author

simap commented Jul 26, 2018

playing around with the test program, I can send 1024b payloads with only 2184b of heap free (after payload is allocated) as long as headerToPayload is false.

@simap
Copy link
Contributor Author

simap commented Jul 26, 2018

Hey I think I found it. The code in ClientContext.h calls _datasource->release_buffer(buf, next_chunk); unconditionally, even if there is an error. This causes it to skip over that data, corrupting the frame.

it should look like this:

err_t err = tcp_write(_pcb, buf, next_chunk, TCP_WRITE_FLAG_COPY);
            DEBUGV(":wrc %d %d %d\r\n", next_chunk, will_send, (int) err);
            if (err == ERR_OK) {
                _datasource->release_buffer(buf, next_chunk);
                _written += next_chunk;
                need_output = true;
            } else {
                break;
            }

thats esp8266 core right? now to figure out how to fork and PR :)

@simap simap closed this as completed Jul 26, 2018
@simap
Copy link
Contributor Author

simap commented Jul 26, 2018

esp8266/Arduino#4265

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