Closed Bug 1368446 Opened 2 years ago Closed 2 years ago

Intermittent toolkit/mozapps/extensions/test/browser/browser_cancelCompatCheck.js | application crashed [@ js::CheckEdgeIsNotBlackToGray(JSObject *,JS::Value const &)] after Assertion failure: JS::ValueIsNotGray(dst), at Barrier.h:251


(Core :: JavaScript: GC, defect, critical)

Not set



Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed


(Reporter: intermittent-bug-filer, Assigned: jonco)



(Keywords: assertion, crash, intermittent-failure, Whiteboard: [stockwell fixed])

Crash Data


(2 files)

Keywords: assertion
Attached file stack
This is presumably just the continuation of bug 1365564. Not sure if we should just dupe it over or what.
Component: JavaScript Engine → JavaScript: GC
Duplicate of this bug: 1365564
doing a bunch of retriggers to figure out the root cause:

:jonco- can you help resolve this ASAP?  the rate of failure is very high (42 failures yesterday puts this on track for #1 intermittent)- if there is no time to work on this, then when the retriggers find a root cause I will suggest a backout.
Flags: needinfo?(jcoppeard)
Whiteboard: [stockwell needswork]
This is another problem with our gray marking assertions.  js::CheckedUnwrap ends up calling ExposeObjectToActiveJS but UncheckedUnwrap does not.  EnqueuePromiseReactionJob (on the failing stack) is calling UncheckedUnwrap.  This call has to happen at this point to make our gray marking assertions pass (it would happen later otherwise).
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
Duplicate of this bug: 1370480
(In reply to Geoff Brown [:gbrown] from comment #10)
This assertion in EnqueuePromiseReactionJob is still going off, perhaps being triggered by different tests (e.g. bug 1370480).
As per comment 6 we need to make UncheckedUnwrap call ExposeObjectToActiveJS to make our gray marking assertions work (incremental GC may not have made the target gray yet but we can write it into the slot of a black object).

This is complicated by the fact that we can't call ExposeObjectToActiveJS from within the GC or off the main thread.

The patch adds another unwrap function, UncheckedUnwrapWithoutExpose, which we can use in those situations.

Try push:
Attachment #8874897 - Flags: review?(sphink)
Comment on attachment 8874897 [details] [diff] [review]

Review of attachment 8874897 [details] [diff] [review]:

::: js/src/jsscript.cpp
@@ +1054,5 @@
>  js::ScriptSourceObject&
>  JSScript::scriptSourceUnwrap() const {
> +    // This may be called off the main thread.
> +    return UncheckedUnwrapWithoutExpose(sourceObject())->as<ScriptSourceObject>();

Why is it valid to not expose here? Maybe because this is never handed back to the embedding? Needs a comment, at least.

::: js/src/proxy/Wrapper.cpp
@@ +334,5 @@
> +
> +    // Eagerly unmark gray wrapper targets so we can assert that we don't create
> +    // black to gray  edges. An incremental GC will eventually  mark the targets
> +    // of black wrappers black  but while it is in progress  we can observe gray
> +    // targets. Expose rather than returning a gray object in this case.

It looks like there are doubled spaces scattered through the comment.
Attachment #8874897 - Flags: review?(sphink) → review+
Pushed by
Fix gray marking assertion failures by calling ExposeObjectToActiveJS in UncheckedUnwrap r=sfink
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Whiteboard: [stockwell needswork] → [stockwell fixed]
You need to log in before you can comment on or make changes to this bug.