Closed Bug 1229970 Opened 4 years ago Closed 4 years ago

Crash in `anonymous namespace''::ScriptLoaderRunnable::OnStartRequest while dragging tabs to their own window

Categories

(Core :: DOM: Service Workers, defect, critical)

44 Branch
x86_64
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox43 --- disabled
firefox44 --- fixed
firefox45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: tmay, Assigned: bkelly)

References

Details

(Keywords: crash, Whiteboard: [betabreakers-fx44])

Crash Data

Attachments

(7 files)

Attached image All Tabs Crashed.jpg
Actual Result:
While pulling tabs out into their own window, all tabs crashed. 

Expected Result:
The user should be able to pull tabs out without crashing the tabs. 

Steps to Recreate:
1. Launch firefox and navigate to alexa top sites. 
2. Click on Cambodia
3. Ctrl click all the links and open the link tabs. 
4. Start using the webpages as you would regularly use them. 
5. Start pulling tabs out and pluggin them back in to the main window. 
6. Observe the tabs all crash. 

Note:
Difficulty reproducing.
Whiteboard: [betabreakers-fx44]
Attached file Kal El DxDiag.txt
Attached file Kal El Graphics 1.txt
Attached file Kal El Graphics 2.txt
Please provide crash reports from about:crashes.
Summary: All Tabs Crashed → All tabs crashed while dragging tabs to their own window
Attached file Kal El Crashes.txt
(In reply to Trammel May from comment #5)
> Created attachment 8695911 [details]
> Kal El Crashes.txt

Thanks, next time you can just copy & paste the ID from about:crashes.

Looking at this further it appears to be a low-volume DOM crash:
 0 	xul.dll 	`anonymous namespace'::ScriptLoaderRunnable::OnStartRequest(nsIRequest*, nsISupports*) 	dom/workers/ScriptLoader.cpp
1 	xul.dll 	nsStreamListenerTee::OnStartRequest(nsIRequest*, nsISupports*) 	netwerk/base/nsStreamListenerTee.cpp
2 	xul.dll 	mozilla::net::HttpChannelChild::DoOnStartRequest(nsIRequest*, nsISupports*) 	netwerk/protocol/http/HttpChannelChild.cpp
3 	xul.dll 	mozilla::net::HttpChannelChild::OnStartRequest(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&, unsigned int const&) 	netwerk/protocol/http/HttpChannelChild.cpp
4 	xul.dll 	mozilla::net::HttpChannelChild::RecvOnStartRequest(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&, unsigned int const&) 	netwerk/protocol/http/HttpChannelChild.cpp
5 		@0xab7 	
6 	xul.dll 	mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PContentChild.cpp
7 	xul.dll 	mozilla::ipc::MessageChannel::DispatchMessageW(IPC::Message const&) 	ipc/glue/MessageChannel.cpp
8 	xul.dll 	mozilla::ipc::MessageChannel::OnMaybeDequeueOne() 	ipc/glue/MessageChannel.cpp
9 	xul.dll 	MessageLoop::DoWork() 	ipc/chromium/src/base/message_loop.cc
10 	ntdll.dll 	LdrpFreeLoadContext 	
11 		@0xb0417f 	

More reports: https://crash-stats.mozilla.com/report/list?product=Firefox&signature=%60anonymous+namespace%27%27%3A%3AScriptLoaderRunnable%3A%3AOnStartRequest
Severity: normal → critical
Component: Graphics → DOM
Keywords: crash
Summary: All tabs crashed while dragging tabs to their own window → Crash in `anonymous namespace''::ScriptLoaderRunnable::OnStartRequest while dragging tabs to their own window
Crash Signature: [@ `anonymous namespace''::ScriptLoaderRunnable::OnStartRequest]
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #6)
> (In reply to Trammel May from comment #5)
> > Created attachment 8695911 [details]
> > Kal El Crashes.txt
> 
> Thanks, next time you can just copy & paste the ID from about:crashes.
> 
> Looking at this further it appears to be a low-volume DOM crash:
>  0 	xul.dll 	`anonymous
> namespace'::ScriptLoaderRunnable::OnStartRequest(nsIRequest*, nsISupports*) 
> dom/workers/ScriptLoader.cpp
> 1 	xul.dll 	nsStreamListenerTee::OnStartRequest(nsIRequest*, nsISupports*) 
> netwerk/base/nsStreamListenerTee.cpp
> 2 	xul.dll 	mozilla::net::HttpChannelChild::DoOnStartRequest(nsIRequest*,
> nsISupports*) 	netwerk/protocol/http/HttpChannelChild.cpp
> 3 	xul.dll 	mozilla::net::HttpChannelChild::OnStartRequest(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&, unsigned int const&) 
> netwerk/protocol/http/HttpChannelChild.cpp
> 4 	xul.dll 	mozilla::net::HttpChannelChild::RecvOnStartRequest(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&, unsigned int const&) 
> netwerk/protocol/http/HttpChannelChild.cpp
> 5 		@0xab7 	
> 6 	xul.dll 	mozilla::dom::PContentChild::OnMessageReceived(IPC::Message
> const&) 	obj-firefox/ipc/ipdl/PContentChild.cpp
> 7 	xul.dll 	mozilla::ipc::MessageChannel::DispatchMessageW(IPC::Message
> const&) 	ipc/glue/MessageChannel.cpp
> 8 	xul.dll 	mozilla::ipc::MessageChannel::OnMaybeDequeueOne() 
> ipc/glue/MessageChannel.cpp
> 9 	xul.dll 	MessageLoop::DoWork() 	ipc/chromium/src/base/message_loop.cc
> 10 	ntdll.dll 	LdrpFreeLoadContext 	
> 11 		@0xb0417f 	
> 
> More reports:
> https://crash-stats.mozilla.com/report/
> list?product=Firefox&signature=%60anonymous+namespace%27%27%3A%3AScriptLoader
> Runnable%3A%3AOnStartRequest

ok, will do
ScriptLoaderRunnable::OnStartRequest() only seems to execute if its loading and caching a service worker script.  This is currently disabled on beta.  This is confusing to me.
(In reply to Ben Kelly [:bkelly] from comment #8)
> ScriptLoaderRunnable::OnStartRequest() only seems to execute if its loading
> and caching a service worker script.  This is currently disabled on beta. 
> This is confusing to me.

Is it possible for someone to pref this on in Beta?
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #9)
> Is it possible for someone to pref this on in Beta?

It is.  I guess it depends on how low volume this really is.  I would expect exceptionally low volume on beta.
I would say volume is "exceptionally" low. Here are the numbers to the last week of data:

Firefox 45.0a1 - 17 crashes
Firefox 44.0a2 - 6 crashes
Firefox 42.0b1 - 5 crashes
Firefox 43.0a1 - 1 crash
Firefox 42.0a2 - 1 crash
It appears https://www.khmerload.com is registering a service worker to handle push notifications.
Component: DOM: Workers → DOM: Service Workers
I see both e10s and non-e10s channels in the crashes.  So it affects both configurations.
I have a theory here, although I have not been able to reproduce.
Assignee: nobody → bkelly
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Trammel, does this patch fix the issue for you?

If you don't want to do your own build, I think you can get a binary from this try once it finishes:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d92ad981028
Attachment #8696709 - Flags: feedback?(tmay)
Comment on attachment 8696709 [details] [diff] [review]
Abort script loading start requests if a load has been canceled. r=khuey

This is still a bit speculative, but I think its a safe and correct change.
Attachment #8696709 - Flags: review?(khuey)
Comment on attachment 8696709 [details] [diff] [review]
Abort script loading start requests if a load has been canceled. r=khuey

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

Seems reasonable.
Attachment #8696709 - Flags: review?(khuey) → review+
We should consider uplifting this to 44 beta if it fixes the problem.
https://hg.mozilla.org/mozilla-central/rev/286ad5849931
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
There have been 3 crashes on 44, but none on 45 since this landed.  I think we should uplift it to 44 beta.
Comment on attachment 8696709 [details] [diff] [review]
Abort script loading start requests if a load has been canceled. r=khuey

Approval Request Comment
[Feature/regressing bug #]: Service workers
[User impact if declined]: Low frequency crash, but it may become higher frequency as more sites start using service workers.
[Describe test coverage new/current, TreeHerder]: Mainly observing crashstats after landing this change.  Its so low frequency we never reproduced it locally.
[Risks and why]: Minimal.  This is a very small change and only affects service workers.
[String/UUID change made/needed]:  None.
Attachment #8696709 - Flags: feedback?(tmay) → approval-mozilla-beta?
Note, I want to land this on 44.  I think that will be beta very soon, thats why I requested the beta flag.  If we can land it sooner on aurora before the merge to beta, that would be even better.
Comment on attachment 8696709 [details] [diff] [review]
Abort script loading start requests if a load has been canceled. r=khuey

Crash fix, taking it in Beta44.
Attachment #8696709 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I'm getting conflicts trying to uplift this to beta 44 in ScriptLoader.cpp.
Can we get a rebased patch for this?
Flags: needinfo?(bkelly)
Attached patch beta patchSplinter Review
Here is a beta patch.  Ritu asked me to post here instead of pushing it.
Flags: needinfo?(bkelly)
Depends on: 1233171
You need to log in before you can comment on or make changes to this bug.