[Curl] CurlRequestScheduler doesn't terminate worker thread in a certain situation.

• Component: Platform
• Status: RESOLVED FIXED

[Takashi Komori]

When a CurlRequest receives didReceiveDataCallback from libcurl, it append a task for main thread by calling callClient.
If CurlRequest::invalidateClient is called before the task is processed, the task will not be processed properly.
This is because invalidateClient changes m_client to nullptr and the task appended by CurlRequest::callClient checks if m_client is null.

When calling CurlRequest::didReceiveData task is not processed properly, a paused connection could remain.
In this case CurlRequestScheduler never releases the client related to it.
Eventually this causes that CurlRequestScheduler's worker thread keeps working.

This CurlRequest::invalidateClient() is called from the destructor of NetworkDataTaskCurl.
To avoid this situation, we should cancel the request before invalidate.

[Takashi Komori]

Created attachment 372442 [details]
Patch

[Fujii Hironori]

(In reply to Takashi Komori from comment #0)
> When calling CurlRequest::didReceiveData task is not processed properly, a
> paused connection could remain.

Why can it remain?

> In this case CurlRequestScheduler never releases the client related to it.

Really? It seems that CurlRequestScheduler calls completeTransfer.

[WebKit Commit Bot]

Comment on attachment 372442 [details]
Patch

Clearing flags on attachment: 372442

Committed r246588: <https://trac.webkit.org/changeset/246588>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/51896162>

[Takashi Komori]

(In reply to Fujii Hironori from comment #2)
> (In reply to Takashi Komori from comment #0)
> > When calling CurlRequest::didReceiveData task is not processed properly, a
> > paused connection could remain.
> 
> Why can it remain?
> 
> > In this case CurlRequestScheduler never releases the client related to it.
> 
> Really? It seems that CurlRequestScheduler calls completeTransfer.

To be precise, the task added by didreceiveData is not processed properly.
And when that task is not processed, the connection rmains paused.


CurlRequest::didReceiveData (PAUSE)
CurlRequest::invokeDidReceiveResponse
|
| (Add task by using callClient)
| 
NetworkDataTaskCurl::curlDidReceiveResponse
NetworkDataTaskCurl::invokeDidReceiveResponse
CurlRequest::completeDidReceiveResponse (UNPAUSE)
CrrlRequest::didCompleteTransfer


In this case, paused connection won't return CURLMSG_DONE message and CurlRequestScheduler::completeTransfer will not be invoked.

[Fujii Hironori]

I got the idea. Thanks.