[e10s] Crash in mozilla::net::HttpChannelChild::OverrideWithSynthesizedResponse with add-on Decentraleyes

RESOLVED FIXED in Firefox 50

Status

()

defect
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: 6lobe, Assigned: valentin.gosu)

Tracking

({crash, regression})

41 Branch
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 wontfix, firefox50 fixed, firefox51 fixed, firefox52 fixed)

Details

(Whiteboard: nightly-community, [necko-active], crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-42d5dd1a-da14-4a6c-9b8c-b20422161028.
=============================================================

STR:

1. Install the addon Decentraleyes from here: https://addons.mozilla.org/en-US/firefox/addon/decentraleyes/

2. Navigate to: https://www.bytemark.co.uk/

3. Refresh the page (you may have to do this a few times).

4. Crash.


Here is the regression range:

22:53.79 INFO: Narrowed nightly regression window from [2015-06-05, 2015-06-08] (3 days) to [2015-06-07, 2015-06-08] (1 days) (~0 steps left)
22:53.79 INFO: Got as far as we can go bisecting nightlies...
22:53.79 INFO: Last good revision: 7d4ab4a9febd (2015-06-07)
22:53.79 INFO: First bad revision: 4700d1cdf489 (2015-06-08)
22:53.79 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7d4ab4a9febd&tochange=4700d1cdf489

I couldnt bisect further because I got some errors.
Crash Signature: [@ RefPtr<T>::assign_with_AddRef | mozilla::net::HttpChannelChild::OverrideWithSynthesizedResponse] → [@ RefPtr<T>::assign_with_AddRef | mozilla::net::HttpChannelChild::OverrideWithSynthesizedResponse] [@ RefPtr<T>::assign_assuming_AddRef | mozilla::net::HttpChannelChild::OverrideWithSynthesizedResponse ]
Has Regression Range: --- → yes
Has STR: --- → yes
Whiteboard: nightly-community
I can't reproduce the crash with FF49 or 52 on Win 7. I installed the add-on, refreshed the page 20x, no crash.


Is the bug only reproducible on Linux?
Keywords: crash
Sorry I forgot to mention this only happens with e10s enabled.

I can only test on Linux. The crash never happens on first page load. Most of the time it happens on second page load.

I guess this could be the source of the issue? https://bugzilla.mozilla.org/show_bug.cgi?id=1164397
Well, just forget my previous comment, I can reproduce it now, but only with e10s enabled like you.
CR FF52:
https://crash-stats.mozilla.com/report/index/55c2304f-1897-407d-9e33-d25042161029

And I confirm the regression range too.
Status: UNCONFIRMED → NEW
Crash Signature: [@ RefPtr<T>::assign_with_AddRef | mozilla::net::HttpChannelChild::OverrideWithSynthesizedResponse] [@ RefPtr<T>::assign_assuming_AddRef | mozilla::net::HttpChannelChild::OverrideWithSynthesizedResponse ] → [@ RefPtr<T>::assign_with_AddRef | mozilla::net::HttpChannelChild::OverrideWithSynthesizedResponse] [@ RefPtr<T>::assign_assuming_AddRef | mozilla::net::HttpChannelChild::OverrideWithSynthesizedResponse ] [@ mozilla::net::HttpChannelChild::OverrideWit…
Ever confirmed: true
Summary: Crash in RefPtr<T>::assign_with_AddRef | mozilla::net::HttpChannelChild::OverrideWithSynthesizedResponse → [e10s] Crash in mozilla::net::HttpChannelChild::OverrideWithSynthesizedResponse
Component: Untriaged → Networking: HTTP
Flags: needinfo?(ehsan)
Keywords: regression
OS: Linux → All
Product: Firefox → Core
Summary: [e10s] Crash in mozilla::net::HttpChannelChild::OverrideWithSynthesizedResponse → [e10s] Crash in mozilla::net::HttpChannelChild::OverrideWithSynthesizedResponse with add-on Decentraleyes
Blocks: 1164397
The problem seems to be that in HttpChannelChild::OnRedirectVerifyCallback we QI mRedirectChannelChild to nsIHttpChannelChild - but in this case mRedirectChannelChild is a DataChannelChild instead.
Not sure exactly what we should do in this case, but it should be easy to avoid the crash.
Josh, any idea what we should do with a DataChannelChild? I assume we should make sure we call CompleteRedirectSetup on it, right?
Flags: needinfo?(josh)
Yeah, looks like https://dxr.mozilla.org/mozilla-central/rev/e3279760cd977aac30bd9e8032d3ee71f55d2a67/netwerk/protocol/http/HttpChannelChild.cpp#1532 should be QIing to nsIChildChannel to ensure we deal properly with data channels.
Flags: needinfo?(josh)
MozReview-Commit-ID: 702H0eJKdx1
Attachment #8806186 - Flags: review?(honzab.moz)
Assignee: nobody → valentin.gosu
Status: NEW → ASSIGNED
Whiteboard: nightly-community → nightly-community, [necko-active]
Comment on attachment 8806186 [details] [diff] [review]
Handle null mNewChannel in OverrideWithSynthesizedResponse

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

I really can't review this, sorry :/
Attachment #8806186 - Flags: review?(honzab.moz) → review?(josh)
Attachment #8806186 - Flags: review?(josh) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/af46e53372abd9a03188e580dbe76ac54f209ed6
Bug 1313740 - Handle null mNewChannel in OverrideWithSynthesizedResponse r=jdm
https://hg.mozilla.org/mozilla-central/rev/af46e53372ab
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment on attachment 8806186 [details] [diff] [review]
Handle null mNewChannel in OverrideWithSynthesizedResponse

Approval Request Comment
[Feature/regressing bug #]: bug 1164397
[User impact if declined]: Crash with the decentraleyes extension installed under e10s when a redirect to a data:// URI occurs
[Describe test coverage new/current, TreeHerder]: currently working on a test.
[Risks and why]: Very low. Just a avoiding a null pointer deref.
[String/UUID change made/needed]: None.
Attachment #8806186 - Flags: approval-mozilla-beta?
Attachment #8806186 - Flags: approval-mozilla-aurora?
Comment on attachment 8806186 [details] [diff] [review]
Handle null mNewChannel in OverrideWithSynthesizedResponse

Fix a crash. Take it in 51 aurora.
Attachment #8806186 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(ehsan)
Comment on attachment 8806186 [details] [diff] [review]
Handle null mNewChannel in OverrideWithSynthesizedResponse

This is in 51, updating 50 uplift request from beta to release
Attachment #8806186 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Comment on attachment 8806186 [details] [diff] [review]
Handle null mNewChannel in OverrideWithSynthesizedResponse

Low risk crash fix, let's include it in 50.1.0
Attachment #8806186 - Flags: approval-mozilla-release? → approval-mozilla-release+
You need to log in before you can comment on or make changes to this bug.