1592747 - Crash in [@ mozilla::net::HttpChannelParent::WaitForBgParent | mozilla::net::HttpChannelParent::ContinueVerification]

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[Marcia Knous [:marcia]]

This bug is for crash report bp-3e356caf-c38d-4c43-bc36-4166c0191026.

Small volume macOS and Windows regression which started in 72 running 20191024214023: https://bit.ly/34bkafO

Possible regression range based on build id: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ad7a152bc66c0d411a6fb0b210d675abed9693c7&tochange=dd7a79a0e83d6d24f672ce3f619f613ca7ab5f1d

Several of the URLs are https://www.reddit.com/r/JUSTNOMIL/

Top 10 frames of crashing thread:

0 xul.dll class RefPtr<mozilla::MozPromise<bool, nsresult, 0> > mozilla::net::HttpChannelParent::WaitForBgParent netwerk/protocol/http/HttpChannelParent.cpp:702
1 xul.dll nsresult mozilla::net::HttpChannelParent::ContinueVerification netwerk/protocol/http/HttpChannelParent.cpp:1006
2 xul.dll mozilla::net::DocumentChannelParent::RedirectToRealChannelFinished netwerk/ipc/DocumentChannelParent.cpp:198
3 xul.dll void mozilla::MozPromise<nsresult, mozilla::ipc::ResponseRejectReason, 1>::ThenValue<`lambda at z:/task_1571951969/build/src/netwerk/ipc/DocumentChannelParent.cpp:579:13', `lambda at z:/task_1571951969/build/src/netwerk/ipc/DocumentChannelParent.cpp:580:13'>::DoResolveOrRejectInternal xpcom/threads/MozPromise.h:726
4 xul.dll nsresult mozilla::MozPromise<bool, nsresult, 0>::ThenValueBase::ResolveOrRejectRunnable::Run xpcom/threads/MozPromise.h:402
5 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1225
6 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:486
7 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:88
8 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:308
9 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290

Comment 1

[Honza Bambas (:mayhemer)]

Matt, does this look like a DocumentChannel regression?

Comment 2

[Matt Woodrow (:mattwoodrow)]

It looks like it probably is.

As far as I can tell, mChannel is nullptr when we try to wait for the bg parent to initialize.

I can't figure out how that would be possible though :(

The only thing I can think of is that DocumentChannelParent::mChannel wasn't HttpBaseChannel, but when we sent the original URI back to the content process, it created HttpChannelChild. That would make HttpChannelParent::ConnectChannel fail silently. I don't see how that's possible though.

There's a few other places we clear mChannel, but I don't think we could hit those while switching channels.

We could add a null check easily, sort of wallpapering over the issue, but I'm not sure what else we can do without STR. Does that sound ok?

Comment 3

[Honza Bambas (:mayhemer)]

I think we redirect to non-http channel somehow:
https://searchfox.org/mozilla-central/rev/b2b0077c2e6a516a76bf8077d6f0237c58f5959a/netwerk/protocol/http/HttpChannelParent.cpp#700

we used to have a bug like this, never figured what was the cause. the crash is gone and I'm afraid I can't provide more info on this one.

Comment 4

[Marcia Knous [:marcia]]

Relatively small crash volume in 72, with a combination of macOS and Windows crashes.

Comment 5

[Andrew Overholt [:overholt]]

Given comment 3 and comment 4, let's mark this as fix-optional for 72 and I'll attach it to the DocumentChannel meta bug so we've got it tracked.

Comment 6

[Matt Woodrow (:mattwoodrow)]

This is what happens when we use view-source with DocumentChannel. The channel in the parent is nsViewSourceChannel, but when we send it to the child, the child creates nsViewSourceChannel+HttpChannelChild. When we try to link the HttpChannelParent with the nsViewSourceChannel it fails.

That said, DocumentChannel is disabled with view-source for this exact reason, and as far as I can tell, you can't redirect to view-source.

Can you think of any other protocols that might behave in a similar way?

Comment 7

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1592747 - Null check mChannel in WaitForBgParent so that we don't crash release builds. r?mayhemer

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ef0dd4cdd3b3
Null check mChannel in WaitForBgParent so that we don't crash release builds. r=mayhemer

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/ef0dd4cdd3b3

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Hi Matt, does this need a Beta approval request?

Comment 11

[Matt Woodrow (:mattwoodrow)]

No, it seems to be specific to DocumentChannel, which isn't enabled for the current beta.