Closed
Bug 1239655
Opened 9 years ago
Closed 9 years ago
Do not do PR_ConnectContinue during shutdown
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: dragana, Assigned: dragana)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
dragana
:
review+
|
Details | Diff | Splinter Review |
There has been couple of shutdown hangs where SocketThread is in PR_ConnectContinue:
https://crash-stats.mozilla.com/report/index/7ed839dd-92fe-4b40-8586-d1abb2160114#allthreads
https://crash-stats.mozilla.com/report/index/32b4adec-87cc-4560-8373-e063c2160114#allthreads
https://crash-stats.mozilla.com/report/index/59eb432d-e0bb-4a8c-98fb-60eab2160114#allthreads
We can skip doing PR_ConnectContinue if we are in shutdown.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8707847 -
Flags: review?(mcmanus)
Assignee | ||
Comment 2•9 years ago
|
||
Comment 3•9 years ago
|
||
Comment on attachment 8707847 [details] [diff] [review]
bug_1239655.patch
Review of attachment 8707847 [details] [diff] [review]:
-----------------------------------------------------------------
::: netwerk/base/nsSocketTransport2.cpp
@@ +1890,5 @@
>
> + // We do not need to do PR_ConnectContinue when we are already
> + // shutting down.
> + if (gIOService->IsNetTearingDown()) {
> + mCondition = NS_ERROR_ABORT;
this is a good find - thank you!
but how does this prevent continue connect from running?
shouldn't isNetTearingDown maybe imply a nspr failed return code for ContinueConnect and the normal logic can run afterwards?
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8707847 -
Attachment is obsolete: true
Attachment #8707847 -
Flags: review?(mcmanus)
Attachment #8707908 -
Flags: review?(mcmanus)
Comment 5•9 years ago
|
||
Comment on attachment 8707908 [details] [diff] [review]
bug_1239655.patch
Review of attachment 8707908 [details] [diff] [review]:
-----------------------------------------------------------------
::: netwerk/base/nsSocketTransport2.cpp
@@ +1963,5 @@
> }
> + else if ((mState == STATE_CONNECTING) && gIOService->IsNetTearingDown()) {
> + // We do not need to do PR_ConnectContinue when we are already
> + // shutting down.
> + mCondition = NS_ERROR_ABORT;
add a socket_log please
Attachment #8707908 -
Flags: review?(mcmanus) → review+
Assignee | ||
Comment 6•9 years ago
|
||
Added a log.
Attachment #8707908 -
Attachment is obsolete: true
Attachment #8707923 -
Flags: review+
Assignee | ||
Comment 7•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Keywords: checkin-needed
Comment 9•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in
before you can comment on or make changes to this bug.
Description
•