Closed Bug 1117873 Opened 7 years ago Closed 7 years ago

Firefox crash with "Enhanced Steam" extension since Fx35b1

Categories

(Core :: Networking: HTTP, defect)

35 Branch
defect
Not set
critical

Tracking

()

RESOLVED DUPLICATE of bug 1123732
Tracking Status
e10s ? ---
firefox35 - unaffected
firefox36 - unaffected
firefox37 - unaffected
firefox38 - affected
relnote-firefox --- 35+

People

(Reporter: yfdyh000, Unassigned)

References

Details

(Keywords: addon-compat, crash, crashreportid)

Crash Data

Run Firefox 35+, install Enhanced Steam extension, open one or more Steam page, the Firefox may crashed.

The crashes seems to be intermittent, I did not find a steps to stable reproduce.


See https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Anet%3A%3AHttpBaseChannel%3A%3ASetupReplacementChannel%28nsIURI*%2C+nsIChannel*%2C+bool%29#tab-comments.
Keywords: addon-compat, crash
[Tracking Requested - why for this release]: the causing crashes.
This affects a good few users:
http://steamcommunity.com/groups/enhancedsteam/discussions/0/624075036457937895/

(and the extension's author has "not been able to pin down the particular issue that's causing Firefox 35 to crash when Enhanced Steam is enabled")

As per the thread, the issue might not exist on Firefox 36.
I can confirm the crashing after upgrading to Firefox 35.
Firefox will crash on any of the "Steam" related web sites with the "Enhanced Steam" extension.
Disabling the extension will restore stability to Firefox on the Steam web sites again.
Never had this issue before with other versions of Firefox or the extension.
I get the crash in firefox 35 and also in seamonkey 2.32, in case that helps narrow it down more.
Crash Signature: [@ mozilla::net::HttpBaseChannel::SetupReplacementChannel(nsIURI*, nsIChannel*, bool) ] → [@ mozilla::net::HttpBaseChannel::SetupReplacementChannel(nsIURI*, nsIChannel*, bool) ] [@ mozalloc_abort(char const* const) | NS_DebugBreak | mozilla::net::PNeckoChild::Write(mozilla::ipc::PrincipalInfo const&, IPC::Message*) ]
Tracking for now, though this may turn out to be a tech evangelism issue, it's not a driver for a 35 dot release as there is a workaround (disabling Enhanced Steam).
also added this to known issues on 35/36/37
Tyler - if there isn't already, would it make sense to put up a support article on disabling this extension for people who wonder about crashes?
Flags: needinfo?(tdowner)
Enhanced Steam developer here.  I believe this is related to XMLHttpRequest calls done from the extension context (pageMod) that are performed on pages which have a 302 temporary redirect.  This could have been introduced by fixing bug #1073882 in Firefox 35, as this behavior works as expected in Firefox 34.

I have been able to reliably fix the issue by removing any XMLHttpRequest calls that are done to redirected files, but this limits some of the extension's functionality and could introduce additional browser crashes if files on the remote server are redirected in the future.

I'm happy to provide any additional information I can.
I can confirm that I experience this crash when using Enhanced Steam 7.2.2 and Firefox 35. There are a large number of users that rely on this add on.
Still crashes occasionally with 7.2.3 version of the extension.
(In reply to jshackles from comment #9)
> Enhanced Steam developer here...
This looks like a nullptr crash, presumably because |uri| is null - meaning one of the redirects has a null URI, which seems odd. mmc/mayhemer, do you have ideas about what's going on here?
Blocks: 1073882
Component: Untriaged → Networking: HTTP
Flags: needinfo?(mmc)
Flags: needinfo?(honzab.moz)
Keywords: crashreportid
STR:
- Nightly debug
- install https://firefox.enhancedsteam.com/latest/enhanced-steam.xpi
- go to http://store.steampowered.com/
- attach the child process

At...

>	xul.dll!mozilla::ipc::PrincipalToPrincipalInfo(0x0a8def24, 0x00240588) Line 132	C++
 	xul.dll!mozilla::net::propagateLoadInfo(0x0d3e4970, {...}) Line 1305	C++
 	xul.dll!mozilla::net::HttpChannelChild::ContinueAsyncOpen() Line 1591	C++
 	xul.dll!mozilla::net::HttpChannelChild::AsyncOpen(0x0d4d9290, 0x00000000) Line 1486	C++
 	xul.dll!nsXMLHttpRequest::Send(0x00000000, {...}) Line 3009	C++
 	xul.dll!nsXMLHttpRequest::Send({...}) Line 423	C++
 	xul.dll!nsXMLHttpRequest::Send(0x04726040, {...}) Line 434	C++
 	xul.dll!nsXMLHttpRequest::Send(0x04726040, {...}, {...}) Line 461	C++
 	xul.dll!mozilla::dom::XMLHttpRequestBinding::send(0x04726040, {...}, 0x0ac4c300, {...}) Line 710	C++
 	xul.dll!mozilla::dom::GenericBindingMethod(0x04726040, 0x00000001, 0x04e1e5c0) Line 2485	C++


0 get_http(url = "http://api.enhancedsteam.com/early_access/", callback = [function]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/loader/sandbox.js -> resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js":285]
    this = [object Window]
1 anonymous() ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/loader/sandbox.js -> resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js":3315]
    this = [object Window]
2 <TOP LEVEL> ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/loader/sandbox.js -> resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js":3306]
    this = [object Window]
3 load(sandbox = [object Window], uri = "resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js") ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/loader/sandbox.js":66]
4 importScripts(workerSandbox = [object Object], urls = resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/jQuery.min.js,resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/localization.js,resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js, "resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/localization.js", "resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js") ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/content/sandbox.js":290]
    this = null
5 WorkerSandbox(worker = [object Object], window = [object Window]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/content/sandbox.js":252]
    this = [object Object]
6 constructor([object Object], [object Window]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/heritage.js":146]
7 initialize(options = [object Object]) ["resource://gre/modules/The thread 0x14bc has exited with code 0 (0x0).


...the aPrincipal is an [nsExpandedPrincipal] having no URI (GetURI returns null but doesn't fail).  Since propagateLoadInfo doesn't check result of mozilla::ipc::PrincipalToPrincipalInfo, we store an empty requestingPrincipalInfo and triggeringPrincipalInfo.  This later fails to serialize and I fail with NS_RUNTIMEABORT("unknown union type") - since it's 0 - at PNeckoChild::Write called as Write((__v).triggeringPrincipalInfo(), __msg) from PNeckoChild::Write(const HttpChannelOpenArgs& __v, Message* __msg).



It's the same reason why we fail to have a URI in the redirect chain later (since it's taken from the same principal).


I see following fixes here:
- fix the add-on to not produce XHR loads w/ url-less principals (?)
- fail the XHR load/creation when principals are url-less (jonas?)
- check URL presence in nsHttpChannel (just a logging code anyway, I can do it right now)
Flags: needinfo?(mmc)
Flags: needinfo?(jshackles)
Flags: needinfo?(jonas)
Flags: needinfo?(honzab.moz)
OS: Windows 8.1 → All
Hardware: x86 → All
Blocks: 1123732
Thanks Honza.
This crashes an extension we've developed as well. Crash Report here: https://crash-stats.mozilla.com/report/index/e57e175b-4477-4741-85c3-ed1302150120 
Since the signature indicates it affects several users, any chance of a dot release with a fix?
Honza, are you able to write a fix for this? This is the last patch we need to do a dot release of 35. Thanks
Flags: needinfo?(honzab.moz)
(In reply to Sylvestre Ledru [:sylvestre] from comment #15)
> Honza, are you able to write a fix for this? This is the last patch we need
> to do a dot release of 35. Thanks

See bug 1123732.
Flags: needinfo?(honzab.moz)
Based on the patch, I tried compiling firefox with --disable-logging and that does seem to make the crash go away, or at least make it much less rare (I haven't seen it at all yet).
I don't know what our security model is on e10s-desktop. Do we trust child processes to use expanded principals? I'm guessing so since they are not that different from "normal" principals?

Mrbkap, can you verify that that is correct?

If so, I think we need to fix PrincipalToPrincipalInfo to be able to properly send Expanded principals across IPC.
Flags: needinfo?(jonas) → needinfo?(mrbkap)
(In reply to Jonas Sicking (:sicking) from comment #18)
> Mrbkap, can you verify that that is correct?
> 
> If so, I think we need to fix PrincipalToPrincipalInfo to be able to
> properly send Expanded principals across IPC.

Yeah, we definitely need to make PrincipalToPrincipalInfo handle expanded principals. Jetpack frame scripts are going to definitely want to do mashups, even in e10s.
Flags: needinfo?(mrbkap)
I no longer have any crashes with (version 7.2.3 of) this extension in Firefox 35.0.1
This is still happening every single time on fx38 (m-c) with e10s enabled. Once you disable e10s, the crash stops.

* Enhanced Steam v7.2.3
* m-c build used: [build id: 20150128101733]
* https://crash-stats.mozilla.com/report/index/5ccd144a-dddc-4eec-af22-d011c2150128
(In reply to Kamil Jozwiak [:kjozwiak] from comment #21)
> This is still happening every single time on fx38 (m-c) with e10s enabled.
> Once you disable e10s, the crash stops.
> 
> * Enhanced Steam v7.2.3
> * m-c build used: [build id: 20150128101733]
> *
> https://crash-stats.mozilla.com/report/index/5ccd144a-dddc-4eec-af22-
> d011c2150128

Can confirm. No crashing anymore with e10s disabled. With e10s enabled going to a steam store page, crashes tab instantly every single time. http://store.steampowered.com/app/218620 for example.
Looks pretty much confirmed and it is even on https://www.mozilla.org/en-US/firefox/36.0beta/releasenotes/ so marking NEW accordingly...
Status: UNCONFIRMED → NEW
Ever confirmed: true
Note: It also occurs in without e10s.
(In reply to YF (Yang) from comment #25)
> Note: It also occurs in without e10s.

Im running latest enhanced steam and nightly i dont experience any crashes without e10s. As far as i know it was supposed to be fixed?
(In reply to kalviskajaks from comment #26)
> (In reply to YF (Yang) from comment #25)
> > Note: It also occurs in without e10s.
> 
> Im running latest enhanced steam and nightly i dont experience any crashes
> without e10s. As far as i know it was supposed to be fixed?

sorry, I forgot the should have been fixed by bug 1123732.
Dropping tracking and marking all versions > 38 as unaffected. 38 will also be unaffected once it moves to Aurora. This needs to be fixed specifically for when e10s is enabled.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(tdowner)
Flags: needinfo?(jshackles)
Resolution: --- → DUPLICATE
Duplicate of bug: 1123732
You need to log in before you can comment on or make changes to this bug.