Bug 198993 - [Curl] CurlRequestScheduler doesn't terminate worker thread in a certain situation.
Summary: [Curl] CurlRequestScheduler doesn't terminate worker thread in a certain situ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-06-19 00:31 PDT by Takashi Komori
Modified: 2019-06-19 21:16 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.53 KB, patch)
2019-06-19 00:56 PDT, Takashi Komori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Takashi Komori 2019-06-19 00:31:09 PDT
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.
Comment 1 Takashi Komori 2019-06-19 00:56:31 PDT
Created attachment 372442 [details]
Patch
Comment 2 Fujii Hironori 2019-06-19 03:38:08 PDT
(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.
Comment 3 WebKit Commit Bot 2019-06-19 08:11:36 PDT
Comment on attachment 372442 [details]
Patch

Clearing flags on attachment: 372442

Committed r246588: <https://trac.webkit.org/changeset/246588>
Comment 4 WebKit Commit Bot 2019-06-19 08:11:38 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Radar WebKit Bug Importer 2019-06-19 08:12:18 PDT
<rdar://problem/51896162>
Comment 6 Takashi Komori 2019-06-19 20:54:18 PDT
(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.
Comment 7 Fujii Hironori 2019-06-19 21:16:42 PDT
I got the idea. Thanks.