WebInputStream: Issue and fix


#1

I’ve had issues with WebInputStream on Linux, fixed them and wanted to share my results. If possible, please integrate with Juce at earliest convenience.

Problem: A streaming connection (i.e. reading a permanent stream from a server and parsing its results - JSON in my case) could not be closed without a !!! killing thread by force !!! assertion, after the connection got stale (broken). The culprit was WebInputStream::read (…), which blocked while waiting for further input forever (or extremely long, at least). Stopping the thread that read and parsed the input was therefore impossible.

I may be missing something, but for me the normal way to implement this is to timeout after a while if nothing was read, wait a little and read/parse/process again until threadShouldExit(). Otherwise there’s no opportunity to stop the thread.

I observed this issue on Linux only. No problems on OS X, where a broken connection causes WebInputStream:: read (…) to timeout and exit properly. This should be the default behavior on all platforms, since a broken connection could otherwise hang and break your app!

In order to make it behave on Linux exactly like on OS X, I’ve added the following 2 lines to juce_curl_Network.cpp, setting CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME (lines 186 ff, annotated with “ans”):

if (timeOutMs > 0)
{
    long timeOutSecs = static_cast<long> (ceil (static_cast<double> (timeOutMs) / 1000.0));

    if (curl_easy_setopt (curl, CURLOPT_CONNECTTIMEOUT, timeOutSecs) != CURLE_OK)
        return false;

    // ans - Important to get a timeout on a stale or broken connection:
    if (curl_easy_setopt (curl, CURLOPT_LOW_SPEED_LIMIT, 100) != CURLE_OK)
        return false;
    if (curl_easy_setopt (curl, CURLOPT_LOW_SPEED_TIME, timeOutSecs) != CURLE_OK)
        return false;                
}

As a result, the connection times out if data throughput goes below 100 Bytes/s for more than the provided timeOutMs. Today, nothing should go below 100 Bytes/s for long, unless something serious is wrong (In order to keep streaming connections alive also when no content is due, servers typically send heartbeat messages). This could probably be set to 1 Byte/s and still work.

I understand that timeOutMs is probably intended for the connection phase only, but since there is no separate parameter for the data transfer phase, it could be used for both. Interestingly enough, that’s equivalent to how it works on OS X.

Two separate timeout parameters would be perfect, but IMO not really necessary. A typical connection timeout might be 10-20 seconds or so, which is also fine for the transfer timeout.

Hope this is helpful.


#2

Great stuff, thanks very much, we’ll take a look!