1505566 - Improve error handling in Stream.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jason Orendorff [:jorendorff]]

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.

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1505566 - Part 1: Handle uncatchable errors correctly in ReadableStream callbacks. r?arai

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1505566 - Part 2: Avoid calling into complicated functions while an exception is pending. r?arai

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

Comment 3

[Pulsebot]

Pushed by [e-mail]:
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

Comment 4

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/b561154d8d5f
https://hg.mozilla.org/mozilla-central/rev/de03aebdab6c