All users were logged out of Bugzilla on October 13th, 2018

Do not use the JSContext to root in parallel code

RESOLVED FIXED in mozilla23

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: terrence, Assigned: terrence)

Tracking

Trunk
mozilla23
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 743754 [details] [diff] [review]
v0

All parallel tests are failing for me currently in an exact rooting build because of push/pop mismatches in the rooting list. I think we should be using the TLS for anything that looks even remotely like parallel code. The parallel tests seem to pass with the attached, at least.
Attachment #743754 - Flags: review?(nmatsakis)
Attachment #743754 - Flags: feedback?(jcoppeard)
Comment on attachment 743754 [details] [diff] [review]
v0

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

Yes, absolutely! This code should have been converted to use the PerThreadData rooting forms long ago, sorry about that.

In many of these cases, the per-thread data is likely available in a ForkJoinSlice* that's hanging around, rather than going through TLS, but basically this looks good.
(Assignee)

Comment 2

6 years ago
Good! I'm glad this is an okay emergency solution. Did you mean to set r+ on the patch?
Attachment #743754 - Flags: review?(nmatsakis) → review+
Comment on attachment 743754 [details] [diff] [review]
v0

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

Ah right, looks like I broke this, sorry about that!  Thanks for the heads up.
Attachment #743754 - Flags: feedback?(jcoppeard)
https://hg.mozilla.org/mozilla-central/rev/a303955db0b5
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.