GetAndClearException in the interpreter calls cx->getPendingException to attempt to convert a pending exception into an object, then clears exceptions, then checks whether getPendingException OOM'd. If an OOM occurs in getPendingException, the OOM message is cleared by clearPendingException and the engine is left unsure of the source of the failure. It's unclear to me why GetAndClearException would need to call clearPendingException before checking the return value of getPendingException, unless this is an artifact of a literal translation from the interpreter code that this function was factored out of: https://hg.mozilla.org/mozilla-central/rev/1ab05052b3b5
Patch attached. I'm having trouble coming up with an isolated test case for this issue. It causes a test to fail with a patch I'm finishing up, but only if run with a non-debug build of SpiderMonkey. oomTest is unfortunately only available in debug builds. Requesting a review from :jandem as the original author of this code.
Attachment #8784593 - Flags: review?(jdemooij)
3 years ago
Assignee: nobody → mismith
Status: NEW → ASSIGNED
Comment on attachment 8784593 [details] [diff] [review] Bug 1297858: Check getPendingException status before clearing exception Review of attachment 8784593 [details] [diff] [review]: ----------------------------------------------------------------- Two reasons for the current code I think: * getPendingException used to be infallible (it was when I added GetAndClearException). * OOM used to be propagated by returning false *without setting any exception on the context*, the current code made more sense in that world. Thanks for fixing, good find!
Attachment #8784593 - Flags: review?(jdemooij) → review+
3 years ago
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/12d8a0ac1520 Check getPendingException status before clearing exception r=jandem
You need to log in before you can comment on or make changes to this bug.