Closed
Bug 882482
Opened 11 years ago
Closed 11 years ago
Don't use the store buffer off the main thread
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: terrence, Assigned: terrence)
References
Details
Attachments
(1 file, 1 obsolete file)
4.81 KB,
patch
|
nmatsakis
:
review+
|
Details | Diff | Splinter Review |
parExtendDenseArray is using HeapSlot::init, which fires post-barriers. The store buffer is not thread-safe so this results in intermittent failures on all parallel tests.
There are probably more of these. Is there a way to assert that we are running on the main thread so that I can find anyone else calling the store buffer off the main thread?
Attachment #761778 -
Flags: review?(nmatsakis)
Assignee | ||
Comment 1•11 years ago
|
||
This adds !InParallelSection assertions. This did turn up the problems I fixed earlier when that patch was not applied. There are no remaining failures with that patch applied.
Attachment #761778 -
Attachment is obsolete: true
Attachment #761778 -
Flags: review?(nmatsakis)
Attachment #761818 -
Flags: review?(nmatsakis)
Comment 2•11 years ago
|
||
Comment on attachment 761818 [details] [diff] [review]
v1
Review of attachment 761818 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/gc/StoreBuffer.h
@@ +498,5 @@
> void releaseVerificationData();
> bool containsEdgeAt(void *loc) const;
>
> + /* We cannot call InParallelSection directly because of a circular dependency. */
> + bool inParallelSection() const;
You could resolve this with a -inl.h file, I suppose. But this is probably easier.
Attachment #761818 -
Flags: review?(nmatsakis) → review+
Assignee | ||
Comment 3•11 years ago
|
||
Yes, -inl.h is a horrid hack that I'd like to avoid if at all possible. Thankfully, there is no problem with out-of-lining here, since it is debug-only.
https://hg.mozilla.org/integration/mozilla-inbound/rev/75575b5f073b
Comment 4•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in
before you can comment on or make changes to this bug.
Description
•