Closed Bug 1058168 Opened 6 years ago Closed 5 years ago

crash in js::PerThreadData::runtimeIfOnOwnerThread()

Categories

(Core :: JavaScript Engine, defect, critical)

34 Branch
x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla34
Tracking Status
firefox33 --- fixed
firefox34 --- fixed

People

(Reporter: lizzard, Assigned: bhackett1024)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-e418d0a8-e3ea-4ceb-8c2f-aaa5d2140823.
=============================================================

We started seeing a lot of crashes with this signature around the 2014060517 build. It looks pretty significant in Firefox 32 Beta. It's the #16 topcrash for Beta with 856/118069 crashes in the last 7 days.  he signature spiked hugely on the 2014071618 build, then went down to almost nothing, then started picking up again between 2014072100 and 2014072209. 7/21 was merge day and I'm not sure how to figure out the regression range there. 

More reports: https://crash-stats.mozilla.com/report/list?signature=js%3A%3APerThreadData%3A%3AruntimeIfOnOwnerThread%28%29&product=Firefox&query_type=contains&range_unit=weeks&process_type=any&version=Firefox%3A32.0b9&hang_type=any&date=2014-08-25+17%3A00%3A00&range_value=1#tab-sigsummary

Crashing thread:
0 	mozjs.dll 	js::PerThreadData::runtimeIfOnOwnerThread() 	js/src/vm/Runtime.h
1 	mozjs.dll 	RuntimeForCurrentThread 	js/src/jit/AsmJSSignalHandlers.cpp
2 	mozjs.dll 	HandleException 	js/src/jit/AsmJSSignalHandlers.cpp
3 	mozjs.dll 	AsmJSExceptionHandler 	js/src/jit/AsmJSSignalHandlers.cpp
4 	ntdll.dll 	NormBuffer__AppendEx 	
5 	ntdll.dll 	RtlCallVectoredExceptionHandlers 	
6 	ntdll.dll 	RtlDispatchException 	
7 	ntdll.dll 	KiUserExceptionDispatcher 	
8 	mozjs.dll 	js::PerThreadData::runtimeIfOnOwnerThread() 	js/src/vm/Runtime.h
EXCEPTION_STACK_OVERFLOW from endless AsmJSExceptionHandler.

The failing piece of runtimeIfOnOwnerThread+0xa is:
cmp     dword ptr [esi+100h],eax ds:0023:00000100=????????

Looks like a null runtime_?
Flags: needinfo?(bhackett1024)
If the runtime is null here we will crash in the signal handler, yeah, then keep recursing until the stack is eventually blown.  It's possible for PerThreadData to have a null runtime, for helper threads operating outside an AutoEnterRuntime.  That can happen in particular during background sweeping, which happens on these threads as of bug 966646.

The attached patch fixes runtimeIfOnOwnerThread() to handle the null runtime case.  This should fix the signal handler recursion, though we did already crash somewhere else in order to enter the signal handler at all (which could be due to random corruption; background sweeping touches a lot of memory).
Attachment #8479284 - Flags: review?(luke)
Flags: needinfo?(bhackett1024)
Attachment #8479284 - Flags: review?(luke) → review+
Comment on attachment 8479284 [details] [diff] [review]
handle null runtime

Approval Request Comment
[User impact if declined]: This patch will improve crash diagnostics by avoiding an infinite recursion in the signal handler used for asm.js and Ion code.
[Risks and why]: none
Attachment #8479284 - Flags: approval-mozilla-aurora?
Attachment #8479284 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Safe to say that we're not doing anything else here (and crash-stats indicates that this is gone in 33+).
Assignee: nobody → bhackett1024
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.