Closed
Bug 867295
Opened 11 years ago
Closed 11 years ago
Do not use the JSContext to root in parallel code
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: terrence, Assigned: terrence)
References
Details
Attachments
(1 file)
(deleted),
patch
|
nmatsakis
:
review+
|
Details | Diff | Splinter Review |
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 1•11 years ago
|
||
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•11 years ago
|
||
Good! I'm glad this is an okay emergency solution. Did you mean to set r+ on the patch?
Updated•11 years ago
|
Attachment #743754 -
Flags: review?(nmatsakis) → review+
Comment 3•11 years ago
|
||
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)
Assignee | ||
Comment 4•11 years ago
|
||
Comment 5•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in
before you can comment on or make changes to this bug.
Description
•