Closed Bug 1505566 Opened 10 months ago Closed 10 months ago

Improve error handling in Stream.cpp

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: jorendorff, Assigned: jorendorff)

References

Details

Attachments

(2 files)

I've written some tests that can cause assertions by forcing an uncatchable error during a ReadableStream callback. Since uncatchable errors are used to implement worker.terminate(), this blocks bug 1387503.

While fixing this, I noticed that we sometimes leave an exception sitting in cx->unwrappedException_ while running complex error-handling code. That's not a good place to store a value that you'll need later; anything could clobber it at any time.
No longer blocks: 1387503
Some standard algorithms detect an error, do some complicated error handling,
then re-throw the error. Before this patch, we implement this by leaving the
exception pending while doing the error handling. Then once the error handling
is done, we simply return false. This is incorrect if any of the intervening
code internally throws any error, then catches and clears it.

The right way to implement the standard is to store the pending exception in a
local variable and explicitly re-throw it later.

Depends on D11235
Blocks: 1387503
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b561154d8d5f
Part 1: Handle uncatchable errors correctly in ReadableStream callbacks. r=arai
https://hg.mozilla.org/integration/autoland/rev/de03aebdab6c
Part 2: Avoid calling into complicated functions while an exception is pending. r=arai
https://hg.mozilla.org/mozilla-central/rev/b561154d8d5f
https://hg.mozilla.org/mozilla-central/rev/de03aebdab6c
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Assignee: nobody → jorendorff
You need to log in before you can comment on or make changes to this bug.