Closed Bug 658864 Opened 9 years ago Closed 9 years ago
The GC can still happen when reporting OOM
The bug 640265 tries to prevent the GC from happening when reporting OOM. Hover, the implemented fix has a race that still allows to run the GC under OOM. The problem comes from using the global boolean JSRuntime::inOOMReport flag that is updated and checked outside a lock. As such the check in the js_GC for the flag can happen after an OOM also happens on another thread which can clear the flag before js_GC accesses it.
There is yet another problem. The error reporter for DOM workers suspends the current request even for OOM error, see http://hg.mozilla.org/tracemonkey/file/57412df720cf/dom/src/threads/nsDOMThreadService.cpp#l630. So what can happen is that the main thread waits in js_GC for a worker request to complete, then the worker gets OOM and suspend the request in the error reporter. Then the GC on the main thread starts resulting in GC during memory allocation. To fix both this and the problem from the comment 0 I plan to use atomic increment/decrement to set the OOM reporting flag and move the flag check after the GC has waited for other threads to suspend the requests.
Assignee: general → igor
Attachment #534304 - Flags: review?(anygregor) → review+
severity rating based on bug 640265
Igor: any progress on this one?
http://hg.mozilla.org/mozilla-central/rev/87cb1807aab9 - I forgot to mark the bug fixed-in-tracemonkey after landing it for some time. Sorry about that.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical?] → [sg:critical?] fixed-in-tracemonkey
Merged to mozilla-central on June 6 so it made mozilla7 but not mozilla6. Luke asserts backporting isn't really necessary here, it's an extremely low probability condition to exploit.
I don't know if this applies to the 1.9.2 branch, but we're unlikely to fix it even if it does.
qa-: does not seem to be anything QA can do to verify this fix
Whiteboard: [sg:critical?] fixed-in-tracemonkey → [sg:critical?] fixed-in-tracemonkey [qa-]
You need to log in before you can comment on or make changes to this bug.