If add() is successful but send() is not, write() will return no bytes written however the bytes still get added in the queue to be sent

[ZongyiYang]

I've been having stability issues when trying to send mjpeg streams to multiple clients. Eventually the mjpeg images would start glitching out and becoming gray, even though bytes are still being sent.

I've found that the issue comes from this line:

AsyncTCP/src/AsyncTCP.cpp

Line 1020 in ca8ac5f

if(!will_send || !send()) {

What happens when large amounts of data are put into the lwip buffer is that you might be able to will_send = add() non-zero frames, but when you call send() the underlying lwip function tcp_output will throw a ERR_MEM error and cause it to return false. That is normally fine and expected behavior if our buffer is full, it just means that lwip is out of memory for new data but we can send it later once it is ready.

However, the AsyncTCP write() function will still return 0 bytes written in this case (add() is non-zero but send() is false). This causes the calling function, such as from ESPAsyncWebServer, to re-write this data, so this data chunk gets added twice (or more) to the buffer and eventually sent.

I'm not sure how you'd like to properly fix it since I don't know if we can recall the bytes added to the queue, but this naive solution fixed all my issues involving sending large amounts of data. Basically just wait until send() actually succeeds:

size_t AsyncClient::write(const char* data, size_t size, uint8_t apiflags) {
    size_t will_send = add(data, size, apiflags);
    if(!will_send) {
        return 0;
    }
    while (connected() && !send()) {
        taskYIELD();
    }
    return will_send;
}

Alternate solutions include just calling send() without the loop, but then the calling functions to write must know that their data might not have actually been sent. Ie: sending the last chunk of data, send() fails, but there is no more calls to send() later on to this client so while data is pushed to the buffer it will never get sent. So basically just moving the same check above outside AsyncTCP and putting the responsibility onto the calling function, but this is a more complicated solution since all the users must modify their write.

[ZongyiYang]

Note: alternate solution is to find a way to undo the add(), but I don't see a lwip function that does that.

Or add the send loop check on connection close maybe.

[stale]

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[stale]

[STALE_DEL] This stale issue has been automatically closed. Thank you for your contributions.