1239655 - Do not do PR_ConnectContinue during shutdown

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Dragana Damjanovic [:dragana]]

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.

Comment 1

[Dragana Damjanovic [:dragana]]

Attachment: bug_1239655.patch

Comment 2

[Dragana Damjanovic [:dragana]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6bde0b1ea2b

Comment 3

[Patrick McManus [:mcmanus]]

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?

Comment 4

[Dragana Damjanovic [:dragana]]

Attachment: bug_1239655.patch

Comment 5

[Patrick McManus [:mcmanus]]

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

Comment 6

[Dragana Damjanovic [:dragana]]

Attachment: bug_1239655.patch

Added a log.

Comment 7

[Dragana Damjanovic [:dragana]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9e546e096bc2

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/34a6488a7b20

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/34a6488a7b20