[whatwg/streams] WritableStreamDefaultController [[strategySizeAlgorithm]] called after being set to undefined (Issue #1218)

Consider the following code:

```js
promise_test(async () => {
  let writer;
  const ws = new WritableStream({
    async close() {
      await flushAsyncEvents();
      writer.write();
    }
  });
  writer = ws.getWriter();
  await writer.close();
}, 'write() during close() should do nothing');
```

The call to `writer.close()` results in a call to `WritableStreamDefaultControllerProcessClose`, which runs `[[closeAlgorithm]]` which calls into the underlying sink close() method in step 5. This immediately returns, executing step 6. `Perform ! WritableStreamDefaultControllerClearAlgorithms(controller).`

On the next microtask, `writer.write()` is called. This results in a call to `WritableStreamDefaultWriterWrite` which on step 4 calls `WritableStreamDefaultControllerGetChunkSize`. This runs `controller.[[strategySizeAlgorithm]]` which was set to undefined in `WritableStreamDefaultControllerClearAlgorithms`.

The reference implementation doesn't crash. Instead, it catches the 'undefined is not a function' error:

```js
  try {
    return controller._strategySizeAlgorithm(chunk);
  } catch (chunkSizeE) {
    WritableStreamDefaultControllerErrorIfNeeded(controller, chunkSizeE);
    return 1;
  }
```

This seems like cheating.

Blink's implementation [explicitly checks if `[[strategySize]]` is undefined](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/streams/writable_stream_default_controller.cc?q=file:third_party%2Fblink%2Frenderer%2Fcore%2Fstreams%2Fwritable_stream_default_controller.cc%20GetChunkSize&ss=chromium%2Fchromium%2Fsrc). It only crashes when compiled with debugging checks. I don't recall why I included a check for undefined there. It also seems like cheating, since it's nowhere in the standard.

I think the correct fix is not to call `WritableStreamDefaultControllerClearAlgorithms` until after the promise returned by `[[closeAlgorithm]]` resolves or rejects. I don't recall why I didn't do that originally.

The same also applies to `[[abortAlgorithm]]`.

What do you think?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/issues/1218
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/streams/issues/1218@github.com>