Closed Bug 1247515 Opened 4 years ago Closed 4 years ago

Intermittent browser_readerMode_hidden_nodes.js | application crashed [@ js::gc::AssertGCThingHasType(js::gc::Cell *,JS::TraceKind)] after Assertion failure: kind == JS::TraceKind::Null, at c:/builds/moz2_slave/m-in-w32-d-0000000000000000000/build/src/js

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox44 --- wontfix
firefox45 --- fixed
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: philor, Assigned: terrence)

References

Details

(Keywords: assertion, intermittent-failure)

Attachments

(1 file)

This is a GC assertion, but it is happening while we are tracing a nsScriptErrorWithStack. Terrence, what does this assertion mean?
Flags: needinfo?(terrence)
It means that we tried to trace a nullptr. The mStack member of nsScriptErrorWithStack is a nullable HeapObject, yet we are using the Value macro to emit CC boilerplate. This elides the null check before the trace, with the expected consequences.
Assignee: nobody → terrence
Status: NEW → ASSIGNED
Flags: needinfo?(terrence)
Attachment #8718399 - Flags: review?(continuation)
Component: JavaScript: GC → XPConnect
Comment on attachment 8718399 [details] [diff] [review]
null_check_mStack_before_tracing-v0.diff

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

Ugly. I should create a little inline function for NS_IMPL_CYCLE_COLLECTION_TRACE_JSVAL_MEMBER_CALLBACK so it only allows values!

Thanks for looking at this.
Attachment #8718399 - Flags: review?(continuation) → review+
Blocks: 1247686
It looks like this is a regression from the initial landing of this class. I have no idea why it hasn't shown up until now.
Blocks: 814497
I can't see any evidence of this on crash stats, but it would still be nice to backport it at least to Aurora as it should be very low risk.
(In reply to Andrew McCreight [:mccr8] from comment #4)
> It looks like this is a regression from the initial landing of this class. I
> have no idea why it hasn't shown up until now.

I'd guess that it's very rare for us not to get a stack when reporting an error. Maybe this only occurs under OOM conditions?
https://hg.mozilla.org/integration/mozilla-inbound/rev/a005f1e07f203bf6a4848ba5e6d75cf53bf9acd4
Bug 1247515 - Check nsScriptErrorWithStack's mStack member for null before tracing; r=mccr8
https://hg.mozilla.org/mozilla-central/rev/a005f1e07f20
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Should we get the fix to branches?
Flags: needinfo?(terrence)
Comment on attachment 8718399 [details] [diff] [review]
null_check_mStack_before_tracing-v0.diff

Approval Request Comment
[Feature/regressing bug #]: old
[User impact if declined]: crashes, but it isn't clear if they are happening for actual users
[Describe test coverage new/current, TreeHerder]: this code runs frequently
[Risks and why]: low, it just adds a null check.
[String/UUID change made/needed]: none
Flags: needinfo?(terrence)
Attachment #8718399 - Flags: approval-mozilla-beta?
Attachment #8718399 - Flags: approval-mozilla-aurora?
Comment on attachment 8718399 [details] [diff] [review]
null_check_mStack_before_tracing-v0.diff

Fix a potential crash, taking it.
Attachment #8718399 - Flags: approval-mozilla-beta?
Attachment #8718399 - Flags: approval-mozilla-beta+
Attachment #8718399 - Flags: approval-mozilla-aurora?
Attachment #8718399 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.