1802471 - Verify if we can bail out earlier from nsHttpChannel::BeginConnect in case of shutdown

Status: RESOLVED FIXED

(Core :: Networking: HTTP, task, P2)

Description

[Jens Stutte [:jstutte]]

In bug 1768581 we've seen that a late creation after net-teardown of an HTTP channel can lead to an unnecessary initialization of the cookie DB (and who knows what else) before it actually refuses to work.

The very beginning of nsHttpChannel::BeginConnect might be the right place to add an AppShutdown::IsInOrBeyond(ShutdownPhase::AppShutdownNetTeardown) check and bail out with an error in case (which?).

Comment 1

[Kershaw Chang [:kershaw]]

(In reply to Jens Stutte [:jstutte] from comment #0)

In bug 1768581 we've seen that a late creation after net-teardown of an HTTP channel can lead to an unnecessary initialization of the cookie DB (and who knows what else) before it actually refuses to work.

The very beginning of nsHttpChannel::BeginConnect might be the right place to add an AppShutdown::IsInOrBeyond(ShutdownPhase::AppShutdownNetTeardown) check and bail out with an error in case (which?).

I think we can check gHttpHandler->Active() before calling BeginConnect.

Comment 2

[Jens Stutte [:jstutte]]

Ah, sure, that might work as well. There is a slight difference in time, AppShutdown::IsInOrBeyond(ShutdownPhase::AppShutdownNetTeardown) triggers as soon as we enter that shutdown phase, while gHttpHandler->Active() is flipped as consequence of the net-teardown notification. I assume that there are not many listeners to net-teardown, so the difference is probably neglect-able here and it seems wise to use your existing infrastructure for this.

Comment 3

[Jens Stutte [:jstutte]]

First try push. There might be something to investigate...

Comment 4

[Jens Stutte [:jstutte]]

Attachment: Bug 1802471 - nsHttpChannel::MaybeResolveProxyAndBeginConnect should not call BeginConnect if the handler is not active. r?#necko-reviewers

Comment 5

[Jens Stutte [:jstutte]]

(In reply to Jens Stutte [:jstutte] from comment #3)

First try push. There might be something to investigate...

Just some intermittent failures, hopefully. I am curious about those WPT leaks in a service worker test, but might just land as is.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7cb2bf89ee90
nsHttpChannel::MaybeResolveProxyAndBeginConnect should not call BeginConnect if the handler is not active. r=necko-reviewers,kershaw

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/7cb2bf89ee90