Closed Bug 1289581 Opened 4 years ago Closed 4 years ago

Check all slot assignments for black->gray edges

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: terrence, Assigned: terrence)

References

Details

Attachments

(1 file)

I was hoping this wouldn't be needed because of the slowdown, but it looks like we already call out-of-line, so I don't expect any trouble. Other than new assertion failures. This should help us track down what's going off the rails in bug 1289428.

Try run is up at [1], but I haven't even tried this in a browser yet, so who knows how far that will get.

1- https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d38bb14ec92
Attachment #8774888 - Flags: review?(jcoppeard)
Comment on attachment 8774888 [details] [diff] [review]
assert_no_slot_sets_gray-v0.diff

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

Looks fine.

The non-unified build failure is telling me that you need to include Heap.h though.
Attachment #8774888 - Flags: review?(jcoppeard) → review+
The original try run didn't seem to catch anything so I pushed a bigger one:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=79b1c9b74074
The followup didn't catch anything either, so I pushed a full try run, on top of the other other assertions as well. If the CC-in-JSAPI patch hits but the slot assignment does not, we'll know that something incredibly dangerous is going on.
(In reply to Terrence Cole [:terrence] from comment #3)
There was this: https://treeherder.mozilla.org/logviewer.html#?job_id=24597359&repo=try#L2197
(In reply to Jon Coppeard (:jonco) from comment #5)
> (In reply to Terrence Cole [:terrence] from comment #3)
> There was this:
> https://treeherder.mozilla.org/logviewer.html#?job_id=24597359&repo=try#L2197

Looks like the Promise bug I fixed yesterday.
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8a125a7dc496d5366d4065cc58938da5145d41c
Bug 1289581 - Assert that no object slot gets set with a black to gray edge; r=jonco
https://hg.mozilla.org/mozilla-central/rev/d8a125a7dc49
https://hg.mozilla.org/mozilla-central/rev/104899f933cb
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
As mentioned in bug 1291776, it would be nice to uplift this to Beta so we can have a consistent story across the branches.
Flags: needinfo?(terrence.d.cole)
Comment on attachment 8774888 [details] [diff] [review]
assert_no_slot_sets_gray-v0.diff

Approval Request Comment
[Feature/regressing bug #]: The Cycle Collector.
[User impact if declined]: See risks.
[Describe test coverage new/current, TreeHerder]: 2 weeks on TH.
[Risks and why]: None: this adds a single debug-only assertion. The issue it is tracking has been present since ~Firefox 4. Uplifting this will keep our assertion coverage the same between beta, alpha, and trunk, which will allow us to much more easily notice changes in the orange level of the remaining issues so that we know what further fixes need to be uplifted. Taking this patch has zero risk for the release and will potentially let us fix the long-standing issue one release sooner.
[String/UUID change made/needed]: None.
Flags: needinfo?(terrence.d.cole)
Attachment #8774888 - Flags: approval-mozilla-beta?
Comment on attachment 8774888 [details] [diff] [review]
assert_no_slot_sets_gray-v0.diff

Should help debug intermittent test failures, Beta50+
Attachment #8774888 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.