Closed
Bug 1263218
Opened 8 years ago
Closed 8 years ago
OOMTest can race with background threads
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: jandem, Assigned: jonco)
References
Details
Attachments
(1 file)
3.93 KB,
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
I'm hitting this with a local (testing) patch. In OOMTest, the code looks like this: HelperThreadState().waitForAllThreads(); ... do { ... js::oom::SimulateOOMAfter(allocation, thread, false); ... JS_CallFunction(...); ... } while (handledOOM); JS_CallFunction can end up starting new background jobs, and the next loop iteration SimulateOOMAfter will modify oom::maxAllocations. This can race when the background thread is in an AutoEnterOOMUnsafeRegion section (we will fail the assert in ~AutoEnterOOMUnsafeRegion).
Reporter | ||
Comment 1•8 years ago
|
||
Adding a call to waitForAllThreads after JS_CallFunction seems to fix this for me, but not sure if that's the best fix.
Flags: needinfo?(jcoppeard)
Assignee | ||
Comment 2•8 years ago
|
||
Nice catch. This could be what's causing bug 1235677.
Flags: needinfo?(jcoppeard)
See Also: → 1235677
Assignee | ||
Comment 3•8 years ago
|
||
This mainly waits for the helper threads in ResetSimulatedOOM if we were simulating OOM on a background thread. I tested that this didn't noticeably slow down the tests.
Assignee: nobody → jcoppeard
Attachment #8740067 -
Flags: review?(terrence)
Comment 4•8 years ago
|
||
Comment on attachment 8740067 [details] [diff] [review] bug1263218-oom-test-race Review of attachment 8740067 [details] [diff] [review]: ----------------------------------------------------------------- Great find!
Attachment #8740067 -
Flags: review?(terrence) → review+
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fd676a6ad20b
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•