nmattisson/HttpClient

Race condition when no data immediately available but client disconnects

Opened this issue · 0 comments

First off, thank you for such a great HttpClient library! Using content-length has sped up HTTP requests in our projects quite a bit, especially on slower networks where we had to use slightly larger timeouts.

There appears to be a race condition present between the check for client.available() and and check for client.connected().

Symptom: Requests to a server would read the headers and up to about 20 bytes of the body, but would return error (-1) prematurely.

I debugged it and saw that client.available() returned false when the read buffer was empty though more data was expected. The subsequent call to client.connected() would then return false; 'timeout', 'error', and 'done' were all false at that time. Calling client.available() after that loop returned true. So data was placed in the buffer after client.available() returned false, but before client.disconnected() was called.

I commented out the call to client.connected() in the outer loop and it became rock-solid (relying on the timeout to break):

} while (/*client.connected() && */!timeout && !error && !done);
One possible fix is to call client.available() in the outer loop:

} while (client.available() || (client.connected() && !timeout && !error && !done));
To ensure it completely drains the read buffer even after the connection is closed.