Closed Bug 882482 Opened 7 years ago Closed 7 years ago

Don't use the store buffer off the main thread

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: terrence, Assigned: terrence)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch v0 (obsolete) — 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)
Attached patch v1Splinter Review
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 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+
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
https://hg.mozilla.org/mozilla-central/rev/75575b5f073b
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.