Closed
Bug 1067346
Opened 9 years ago
Closed 9 years ago
content process crash in mozilla::net::HttpBaseChannel::DoApplyContentConversions
Categories
(Core :: Networking: HTTP, defect)
Tracking
()
RESOLVED
FIXED
mozilla35
Tracking | Status | |
---|---|---|
firefox32 | --- | unaffected |
firefox33 | --- | unaffected |
firefox34 | + | fixed |
firefox35 | --- | fixed |
People
(Reporter: kairo, Assigned: dragana)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file, 1 obsolete file)
2.89 KB,
patch
|
dragana
:
review+
lmandel
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-f3b08aa5-27fe-4e50-beb9-dfe1c2140915. ============================================================= This started spiking on Nightly 35 in the 20140911064110 build, so the regression is in 9/10 code. Top frames: 0 xul.dll mozilla::net::HttpBaseChannel::DoApplyContentConversions(nsIStreamListener*, nsIStreamListener**, nsISupports*) netwerk/protocol/http/HttpBaseChannel.cpp 1 xul.dll mozilla::net::HttpChannelChild::OnStartRequest(tag_nsresult const&, mozilla::net::nsHttpResponseHead const&, bool const&, mozilla::net::nsHttpHeaderArray const&, bool const&, bool const&, unsigned int const&, nsCString const&, nsCString const&, mozilla::net::NetAddr const&, mozilla::net::NetAddr const&) netwerk/protocol/http/HttpChannelChild.cpp 2 xul.dll mozilla::net::HttpChannelChild::RecvOnStartRequest(tag_nsresult const&, mozilla::net::nsHttpResponseHead const&, bool const&, mozilla::net::nsHttpHeaderArray const&, bool const&, bool const&, unsigned int const&, nsCString const&, nsCString const&, mozilla::net::NetAddr const&, mozilla::net::NetAddr const&, short const&) netwerk/protocol/http/HttpChannelChild.cpp 3 xul.dll mozilla::net::PHttpChannelChild::OnMessageReceived(IPC::Message const&) obj-firefox/ipc/ipdl/PHttpChannelChild.cpp 4 xul.dll mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) obj-firefox/ipc/ipdl/PContentChild.cpp 5 xul.dll mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) ipc/glue/MessageChannel.cpp 6 xul.dll mozilla::ipc::MessageChannel::DispatchMessageW(IPC::Message const&) ipc/glue/MessageChannel.cpp 7 xul.dll mozilla::ipc::MessageChannel::OnMaybeDequeueOne() ipc/glue/MessageChannel.cpp It looks like all the addresses are 0x0, on Windows it's EXCEPTION_ACCESS_VIOLATION_READ, so I guess this might be a null deref. See stats and more reports at https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Anet%3A%3AHttpBaseChannel%3A%3ADoApplyContentConversions%28nsIStreamListener%2A%2C%20nsIStreamListener%2A%2A%2C%20nsISupports%2A%29
![]() |
Reporter | |
Comment 1•9 years ago
|
||
This is the first build after bug 1064885 so I guess any regression range I can get will point to this as the cause and this is an e10s regression.
![]() |
Reporter | |
Comment 2•9 years ago
|
||
Note that all those crashes are in content processes.
Summary: Nightly crash in mozilla::net::HttpBaseChannel::DoApplyContentConversions → content process crash in mozilla::net::HttpBaseChannel::DoApplyContentConversions
Comment 3•9 years ago
|
||
In the mDivertingToParent branch (http://hg.mozilla.org/mozilla-central/annotate/d070787de8f7/netwerk/protocol/http/HttpChannelChild.cpp#l329) we clear mListener, then we pass it as the first argument to DoApplyContentConversions. That's not going to work.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → dd.mozilla
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8489457 -
Flags: review?(jduell.mcbugs)
Comment 5•9 years ago
|
||
Comment on attachment 8489457 [details] [diff] [review] bug-1067346.patch >+ if (*aNewNextListener) { >+ NS_ADDREF(*aNewNextListener); >+ } This can just be NS_IF_ADDREF(*aNewNextListener);
Comment 6•9 years ago
|
||
Comment on attachment 8489457 [details] [diff] [review] bug-1067346.patch Review of attachment 8489457 [details] [diff] [review]: ----------------------------------------------------------------- +1 with jdm's nit.
Attachment #8489457 -
Flags: review?(jduell.mcbugs) → review+
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8489457 -
Attachment is obsolete: true
Attachment #8490016 -
Flags: review+
Assignee | ||
Comment 8•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=e7e79dfabdd2
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 9•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b1531c87c48
Keywords: checkin-needed
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3b1531c87c48
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Assignee | ||
Comment 11•9 years ago
|
||
this bug is introduced with patch from Bug 1043256, they should be applied at the same time
Flags: needinfo?(ryanvm)
Assignee | ||
Comment 13•9 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #12) > Is there a question for me here? target milestone is different. and bug 1043256 depends on this one.
Comment 14•9 years ago
|
||
Target milestone tracks when the patch landed on m-c. If you want this landed on 34, you need to fill out an Aurora approval request like any other patch. I'm taking care of setting the status flags for you. In the future, it'd be helpful if you did so yourself. Also setting the proper dependencies is useful.
Blocks: 1043256
status-firefox34:
--- → affected
status-firefox35:
--- → fixed
tracking-firefox34:
--- → ?
Assignee | ||
Comment 15•9 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #14) > Target milestone tracks when the patch landed on m-c. If you want this > landed on 34, you need to fill out an Aurora approval request like any other > patch. I'm taking care of setting the status flags for you. In the future, > it'd be helpful if you did so yourself. Also setting the proper dependencies > is useful. thanks a lot
Updated•9 years ago
|
Comment 16•9 years ago
|
||
Comment on attachment 8490016 [details] [diff] [review] fix v2 Approval Request Comment [Feature/regressing bug #]: This needs to land in the same places as bug 1043256 or that fix may cause crashes in e10s occasionally (clearly not that often, since 1043256 has already landed in b2g 2.1 and the world hasn't ended. But we should fix). Sadly I'm not clear on which flags are needed to ask for B2G 2.1 approval--do I need something more than approval-aurora+? [Describe test coverage new/current, TBPL]: Crash-stats is not showing me any crashes for builds after this landed. [Risks and why]: very low--just change to return early before we might deref a variable that could possibly be null. [String/UUID change made/needed]: none
Attachment #8490016 -
Flags: approval-mozilla-aurora?
Comment 17•9 years ago
|
||
Comment on attachment 8490016 [details] [diff] [review] fix v2 (In reply to Jason Duell (:jduell) from comment #16) > Approval Request Comment > [Feature/regressing bug #]: This needs to land in the same places as bug > 1043256 or that fix may cause crashes in e10s occasionally (clearly not that > often, since 1043256 has already landed in b2g 2.1 and the world hasn't > ended. But we should fix). Given that e10s is disabled by default in 34, I think this means that Firefox 34 is unaffected. Can you please confirm? > Sadly I'm not clear on which flags are needed to ask for B2G 2.1 > approval--do I need something more than approval-aurora+? You've done the right thing. B2G 2.1 currently requires aurora approval. Aurora+
Attachment #8490016 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(jduell.mcbugs)
Comment 19•9 years ago
|
||
> Given that e10s is disabled by default in 34, I think this means that Firefox 34
> is unaffected. Can you please confirm?
AFAIK that's true, but dragana should know for sure.
Flags: needinfo?(jduell.mcbugs) → needinfo?(dd.mozilla)
Assignee | ||
Comment 20•9 years ago
|
||
(In reply to Jason Duell (:jduell) from comment #19) > > Given that e10s is disabled by default in 34, I think this means that Firefox 34 > > is unaffected. Can you please confirm? > > AFAIK that's true, but dragana should know for sure. it is true, Firefox 34 with e10s disable is unaffected
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(dd.mozilla)
You need to log in
before you can comment on or make changes to this bug.
Description
•