WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
224191
REGRESSION(
r267763
) NetworkProcess never terminates
https://bugs.webkit.org/show_bug.cgi?id=224191
Summary
REGRESSION(r267763) NetworkProcess never terminates
Alex Christensen
Reported
2021-04-05 10:15:17 PDT
REGRESSION(
r267763
) NetworkProcess never terminates
Attachments
Patch
(9.11 KB, patch)
2021-04-05 10:19 PDT
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Patch
(10.06 KB, patch)
2021-04-05 11:32 PDT
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Patch
(10.81 KB, patch)
2021-04-05 11:54 PDT
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Alex Christensen
Comment 1
2021-04-05 10:19:35 PDT
Created
attachment 425168
[details]
Patch
Alex Christensen
Comment 2
2021-04-05 10:19:38 PDT
<
rdar://problem/76124590
>
Chris Dumez
Comment 3
2021-04-05 10:37:09 PDT
Comment on
attachment 425168
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=425168&action=review
> Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:1289 > + terminate();
Why are we calling terminate() instead of clearing the defaultNetworkProcess (which should deallocate the NetworkProcessProxy and cause the process to go away)? While both are fairly similar, I think clearing the default network process might be nicer for a few reasons: 1. This is what we used to do. When destroying the WebProcessPool, we would deallocate the NetworkProcessProxy and it would just go away (no termination involved) 2. There is a slight risk that there could be code somewhere with a RefPtr<> to a NetworkProcess (e.g. for some kind of transient operation) and terminate would abruptly interrupt the operation instead of letting it finish. 3. Youl will likely run more code doing a termination (due to termination handling) when simple destruction.
> Source/WebKit/UIProcess/WebProcessPool.cpp:381 > + NetworkProcessProxy::defaultNetworkProcess()->terminate();
Ditto.
Alex Christensen
Comment 4
2021-04-05 11:32:07 PDT
Created
attachment 425178
[details]
Patch
Chris Dumez
Comment 5
2021-04-05 11:41:41 PDT
Comment on
attachment 425178
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=425178&action=review
r=me with naming change.
> Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:104 > +RefPtr<NetworkProcessProxy>& NetworkProcessProxy::defaultNetworkProcessPtr()
I think the WebKit pattern is: defaultNetworkProcessIfExists() I don't like the Ptr() naming. When I saw it, it really wasn't clear to me what it meant. If you don't like defaultNetworkProcessIfExists(), I guess we could call this one defaultNetworkProcess() and the one below: ensureDefaultNetworkProcess().
Alex Christensen
Comment 6
2021-04-05 11:54:22 PDT
Created
attachment 425181
[details]
Patch
EWS
Comment 7
2021-04-05 21:03:37 PDT
Committed
r275487
: <
https://commits.webkit.org/r275487
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 425181
[details]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug