Closed Bug 1291776 Opened 9 years ago Closed 8 years ago

Intermittent test_failures_AES-CTR.html,browser_net_statistics-02.js (and many others) | application crashed [@ JSCompartment::wrap(JSContext *,JS::MutableHandle<JSObject *>,JS::Handle<JSObject *>)] after Assertion failure: !ObjectIsMarkedGray(obj)

Categories

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

Product:
Core
Component:
JavaScript: GC
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla51
Tracking Flags:
Tracking Status
firefox49 --- unaffected
firefox50 --- fixed
firefox51 --- fixed

People

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

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

assert_we_dont_create_gray_args-v0.diff
1.12 KB, patch
sfink
: review+
Details | Diff | Splinter Review
Summary: Intermittent /WebCryptoAPI/generateKey/test_failures_AES-CTR.html | application crashed [@ JSCompartment::wrap(JSContext *,JS::MutableHandle<JSObject *>,JS::Handle<JSObject *>)] after Assertion failure: !ObjectIsMarkedGray(obj) → Intermittent /WebCryptoAPI/generateKey/test_failures_AES-CTR.html (and many others) | application crashed [@ JSCompartment::wrap(JSContext *,JS::MutableHandle<JSObject *>,JS::Handle<JSObject *>)] after Assertion failure: !ObjectIsMarkedGray(obj)
Summary: Intermittent /WebCryptoAPI/generateKey/test_failures_AES-CTR.html (and many others) | application crashed [@ JSCompartment::wrap(JSContext *,JS::MutableHandle<JSObject *>,JS::Handle<JSObject *>)] after Assertion failure: !ObjectIsMarkedGray(obj) → Intermittent test_failures_AES-CTR.html,browser_net_statistics-02.js (and many others) | application crashed [@ JSCompartment::wrap(JSContext *,JS::MutableHandle<JSObject *>,JS::Handle<JSObject *>)] after Assertion failure: !ObjectIsMarkedGray(obj)
Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3
The browser_net_statistics-02.js one is really frequent on Win8 on Ash (also available opt-in on Try).
This is coming from PromiseReactionJob. Do we have a tracking bug for these asserts failing when coming from there?
Flags: needinfo?(till)
There was another bug with the same failure that had stacks. I think in that bug the gray object was coming off the JS stack. This is almost certainly coming from gecko somewhere, so I think if we assert in CallArgs::create, we'll find the place. https://treeherder.mozilla.org/#/jobs?repo=try&revision=705fb869f57d
Oh, hey, this isn't a debug build failure, as it first appears! We're actually falling over in stage package in xpc-shell. I'll try to get this locally.
IsUnmarkedGray does not null-check!
While true, we already did an early return if !obj, no?
No, it looks like GCThingIsMarkedGray just grabs the pointer and starts poking: http://searchfox.org/mozilla-central/source/js/public/HeapAPI.h#392 I'm not sure why automation didn't post the new try run. In any case, it is here and looks much better already: https://treeherder.mozilla.org/#/jobs?repo=try&revision=912632f99f54
> No, it looks like GCThingIsMarkedGray just grabs the pointer and starts poking: Yes, but we don't pass it null in JSCompartment::wrap as far as I can tell. That's where the null check I mention in comment 21 is...
Ah! The assertion in this patch is that we do not create CallArgs with any array elements that are initially gray -- e.g. I'm just covering more API surface area with assertions in the hope of catching something. Do you see some more specific avenue we can take to root cause this?
Keywords: leave-open
Could try instrumenting intrinsic_EnqueuePromiseReactionJob to do the check and if it fails dump the JS stack so we can get some idea of how we ended up there...
Attachment #8792916 - Flags: review?(sphink) → review+
(In reply to Boris Zbarsky [:bz] (TPAC) from comment #26) > Could try instrumenting intrinsic_EnqueuePromiseReactionJob to do the check > and if it fails dump the JS stack so we can get some idea of how we ended up > there... Good idea! It looks like the patch I just landed will catch this. Maybe we should print the JS stack from assertion failures in SpiderMonkey regardless? Also, looks like Steve already did a try push with a more relevant assertion: https://treeherder.mozilla.org/#/jobs?repo=try&revision=976744db95b20df7328c1ba32265f99451c05d77&selectedJob=27541746 And it caught something!
The assertion says the (first) gray thing is |resolve|. I'm not sure what this tells us. Maybe we know where that goes onto the stack in the first place?
See Also: → 1299780
All the automation failures are on Beta, so this may have been solved on nightly last week and not uplifted to aurora before it became beta. Ryan, I need your magic again! Also, Till is totally off the hook for this one, so I might as well remove his ni.
Flags: needinfo?(ryanvm)
Flags: needinfo?(till)
OK, there were multiple "fixes" here. The AES-CTR wpt failures went away on August 7, which fits when bug 1283634 and bug 1289581 landed (the latter being more relevant here I assume). That didn't actually fix the underlying problem for those tests, it just shifted the signature to bug 1293057. The mochitest failures getting starred under this bug went around around the beginning of September. I'm 99% sure that whatever fixed those has already been uplifted because Aurora50/Beta50 are seeing nothing but the AES-CTR wpt failures. So I think we should call this one fixed and uplift the patches from the bugs above now that things have stabilized enough that we won't incur an orangemageddon for our trouble. That'll at least give us a consistent set of bugs to track across the various branches.
Assignee: nobody → terrence.d.cole
status-firefox51: affected → fixed
status-firefox52: affected → ---
Depends on: 1289581, 1293057
Flags: needinfo?(ryanvm)
Target Milestone: --- → mozilla51
I agree! Thanks for doing the legwork!
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Try push to hopefully confirm the shifted signature: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d6d11bf08d4
(In reply to Ryan VanderMeulen [:RyanVM] from comment #36) > Try push to hopefully confirm the shifted signature: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d6d11bf08d4 Confirmed.
See Also: 1299780 →
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: