WritableStreamDefaultController [[strategySizeAlgorithm]] called after being set to undefined

[ricea]

trafficstars

Consider the following code:

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:

  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. 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?

[domenic]

Looking at the call sites for WritableStreamDefaultControllerClearAlgorithms and I can't really see any rhyme or reason behind them. I'd expect them to happen whenever we set [[state]] to "errored" (maybe "erroring"?) and "closed".

Patching the immediate problem by moving that particular case later might be reasonable, but I wonder if we should do a more comprehensive reform of when they are cleared.

[ricea]

The issue is not urgent (at least for Blink), so we can take the time to do it properly.