The default bug view has changed. See this FAQ.

crash in mozilla::net::nsHTTPCompressConv::OnStopRequest

RESOLVED FIXED in Firefox 46

Status

()

Core
Networking: HTTP
--
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: philipp, Assigned: mcmanus)

Tracking

({crash, regression})

45 Branch
mozilla48
x86
Windows NT
crash, regression
Points:
---

Firefox Tracking Flags

(firefox45 affected, firefox46 fixed, firefox47 fixed, firefox48 fixed)

Details

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

Attachments

(1 attachment)

(Reporter)

Description

a year ago
This bug was filed from the Socorro interface and is 
report bp-61c234c5-ffc4-47c1-bc90-260fd2160220.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::net::nsHTTPCompressConv::OnStopRequest(nsIRequest*, nsISupports*, nsresult) 	netwerk/streamconv/converters/nsHTTPCompressConv.cpp
1 	browsercomps.dll 	nsFeedSniffer::ConvertEncodedData(nsIRequest*, unsigned char const*, unsigned int) 	browser/components/feeds/nsFeedSniffer.cpp
2 	xul.dll 	js::SavedFrame::finishSavedFrameInit(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>) 	js/src/vm/SavedStacks.cpp
3 	xul.dll 	nsRange::AutoInvalidateSelection::~AutoInvalidateSelection() 	dom/base/nsRange.cpp
4 	browsercomps.dll 	nsFeedSniffer::GetMIMETypeFromContent(nsIRequest*, unsigned char const*, unsigned int, nsACString&) 	browser/components/feeds/nsFeedSniffer.cpp
5 	xul.dll 	NS_SniffContent(char const*, nsIRequest*, unsigned char const*, unsigned int, nsACString_internal&) 	netwerk/base/nsNetUtil.cpp
6 	xul.dll 	mozilla::net::CallTypeSniffers 	netwerk/protocol/http/nsHttpChannel.cpp
7 	xul.dll 	CallPeekFunc 	netwerk/base/nsInputStreamPump.cpp
8 	xul.dll 	mozilla::net::CacheFileInputStream::ReadSegments(nsresult (*)(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*), void*, unsigned int, unsigned int*) 	netwerk/cache2/CacheFileInputStream.cpp
9 	xul.dll 	nsInputStreamPump::PeekStream(void (*)(void*, unsigned char const*, unsigned int), void*) 	netwerk/base/nsInputStreamPump.cpp
10 	xul.dll 	CallPeekFunc 	netwerk/base/nsInputStreamPump.cpp
11 	xul.dll 	mozilla::net::nsHttpChannel::ContinueOnStartRequest2(nsresult) 	netwerk/protocol/http/nsHttpChannel.cpp

this signature is first regressing in 44 and seems to be related to brotli compression that landed with bug 366559.
(Assignee)

Updated

a year ago
Assignee: nobody → mcmanus
Whiteboard: [necko-active]
(Assignee)

Comment 1

a year ago
a fairly big brotli change landed in 46 and this signature changed:

https://crash-stats.mozilla.com/report/index/71446acd-54e1-49a1-ae3b-9f04c2160322
https://crash-stats.mozilla.com/report/index/0df6a8e5-dfb0-4133-a1c9-a11822160326
https://crash-stats.mozilla.com/report/index/fab649db-7a1b-4a6d-aff4-2e2182160328
https://crash-stats.mozilla.com/report/index/fc1222ec-333c-45f0-8e2b-900472160330
https://crash-stats.mozilla.com/report/index/1a423587-2f51-49e5-bc1f-6244b2160327

the source of that is pretty clear - mBrotli is lazily created in the first ondataavailable.. so we just need to null check it.

Its not clear to me if this is just covering up the original signature or not - the change in 46 is too big to really ascertain.
(Assignee)

Comment 2

a year ago
Created attachment 8737310 [details] [diff] [review]
make sure brotli context is created in onstoprequest
Attachment #8737310 - Flags: review?(daniel)
(Assignee)

Comment 3

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b55795160f84
Attachment #8737310 - Flags: review?(daniel) → review+
(Assignee)

Comment 4

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce36169cf980f3674fb729a5a528c589974b4bc6
Bug 1261318 - make sure brotli context is created in onstoprequest r=bagder

Comment 5

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ce36169cf980
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox48: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
(Assignee)

Comment 6

a year ago
Comment on attachment 8737310 [details] [diff] [review]
make sure brotli context is created in onstoprequest

crash fix

Approval Request Comment
[Feature/regressing bug #]: brotli support in 44
[User impact if declined]: corner case null deref (when connection is lost after declaring brotli but before sending any encoded data)
[Describe test coverage new/current, TreeHerder]: regression coverage is ok - nothing new
[Risks and why]: very low - literally a null check
[String/UUID change made/needed]:
Attachment #8737310 - Flags: approval-mozilla-beta?
Attachment #8737310 - Flags: approval-mozilla-aurora?
Comment on attachment 8737310 [details] [diff] [review]
make sure brotli context is created in onstoprequest

Crash fix, affects beta, ok to uplift to aurora and beta.
Attachment #8737310 - Flags: approval-mozilla-beta?
Attachment #8737310 - Flags: approval-mozilla-beta+
Attachment #8737310 - Flags: approval-mozilla-aurora?
Attachment #8737310 - Flags: approval-mozilla-aurora+

Comment 8

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/00ab94f6a5bc
status-firefox47: affected → fixed

Comment 9

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/a76c70403f87
status-firefox46: affected → fixed
You need to log in before you can comment on or make changes to this bug.