1429542 - Crash in mozilla::net::HttpChannelChild::Cancel

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is
report bp-b1b3aba6-d058-4a3c-97b5-05ae10180110.
=============================================================

Seen while looking at nightly crash data: http://bit.ly/2CQ5EwO and seem to date back at least to 12-25 (crashes prior to 12-24 have been deleted)

URLs show up, and are mostly from the same sky.com site: https://news.sky.com/video/would-a-latte-levy-reduce-waste-11196296 

Top 10 frames of crashing thread:

0 xul.dll mozilla::net::HttpChannelChild::Cancel netwerk/protocol/http/HttpChannelChild.cpp:2285
1 xul.dll mozilla::net::HttpChannelChild::OverrideRunnable::Run netwerk/protocol/http/HttpChannelChild.cpp:1503
2 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1040
3 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:97
4 xul.dll mozilla::ipc::MessagePumpForChildProcess::Run ipc/glue/MessagePump.cpp:301
5 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:319
6 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:299
7 xul.dll nsBaseAppShell::Run widget/nsBaseAppShell.cpp:157
8 xul.dll nsAppShell::Run widget/windows/nsAppShell.cpp:344
9 xul.dll XRE_RunAppShell toolkit/xre/nsEmbedFunctions.cpp:877

=============================================================

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Pretty sure I introduced this in bug 1204254.  Probably just needs a nullptr check.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Make HttpChannelChild::OverrideRunnable check for nullptr before using mNewChannel. r=asuth

Andrew, given that other code does nullptr checks for mNewChannel I think its clear this code should be doing it as well.  I think I introduced this in bug 1204254.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8941557 [details] [diff] [review]
Make HttpChannelChild::OverrideRunnable check for nullptr before using mNewChannel. r=asuth

Review of attachment 8941557 [details] [diff] [review]:
-----------------------------------------------------------------

Does this want to get on the uplift bandwagon?

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Yea, I'll request uplift after the tree opens and I can push to inbound at least.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ce7be8b2a72
Make HttpChannelChild::OverrideRunnable check for nullptr before using mNewChannel. r=asuth

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8941557 [details] [diff] [review]
Make HttpChannelChild::OverrideRunnable check for nullptr before using mNewChannel. r=asuth

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1204254 
[User impact if declined]: Low frequency crashes
[Is this code covered by automated tests?]: I don't have exact steps to reproduce, so I do not have a test that specifically triggers this.
[Has the fix been verified in Nightly?]:  Its been pushed to inbound, but is not in nightly yet.
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Minimal risk
[Why is the change risky/not risky?]: This patch simply adds a nullptr check.
[String changes made/needed]: None

Comment 7

[Gerry Chang [:gchang]]

Comment on attachment 8941557 [details] [diff] [review]
Make HttpChannelChild::OverrideRunnable check for nullptr before using mNewChannel. r=asuth

A simple fix for crash. Beta58+.

Comment 8

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/1ce7be8b2a72

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/398360c9d5c6