Assertion failure in ext/web/06_streams.js when stream[_state] === 'closed'

[cscheid]

Version: Deno 1.41.0

Repro:

deno_repro.ts

async function processOutput(iterator: AsyncIterable<Uint8Array>): Promise<string> {
  const decoder = new TextDecoder();
  let outputText = "";
  for await (const chunk of iterator) {
    const text = decoder.decode(chunk);
    outputText += text;
  }
  return outputText;
}

async function* iteratorFromStream(stream: ReadableStream<Uint8Array>) {
  const reader = stream.getReader();
  try {
    while (true) {
      const { done, value } = await reader.read();
      if (done) return;
      yield value;
    }
  } finally {
    reader.releaseLock();
  }
}

const command = new Deno.Command("ls", {stdout: "piped", stderr: "piped"});
const process = command.spawn();

const promises: Promise<unknown>[] = [
  processOutput(iteratorFromStream(process.stdout)),
  processOutput(iteratorFromStream(process.stderr)),
];

await Promise.all(promises);

// assertion fails here
const status = await process.output();

Now run:

$ deno run --allow-all deno_repro.ts
error: Uncaught (in promise) AssertionError: Assertion failed.
const status = await process.output();
               ^
    at assert (ext:deno_web/00_infra.js:374:11)
    at readableStreamError (ext:deno_web/06_streams.js:2533:3)
    at readableStreamCollectIntoUint8Array (ext:deno_web/06_streams.js:1085:7)
    at eventLoopTick (ext:core/01_core.js:166:7)
    at async Promise.all (index 1)
    at async ChildProcess.output (ext:runtime/40_process.js:322:49)
    at async file:///Users/cscheid/Desktop/daily-log/2024/02/28/deno_repro.ts:35:16

Adding some debug print statements around (ext:deno_web/06_streams.js:2533:3) shows that at the point that readableStreamError(stream, err) is called, this stream actually is closed, failing the assertion assert(stream[_state] === "readable"); in line 2533.

The error object err is as follows, although I don't understand why we end with a bad resource ID:

BadResource: Bad resource ID
    at readableStreamCollectIntoUint8Array (ext:deno_web/06_streams.js:1069:23)
    at collectOutput (ext:runtime/40_process.js:221:10)
    at ChildProcess.output (ext:runtime/40_process.js:324:7)
    at file:///Users/cscheid/Desktop/daily-log/2024/02/28/deno_repro.ts:35:30
    at eventLoopTick (ext:core/01_core.js:166:7) {
  name: "BadResource"
}

The ls process does appear to run, and the stdout and stderr strings look correct.

[cscheid]

The problem seems to happen when op_read_all is called with a resourceBacking.rid === 4. In the repro above, that path is called twice, once with resourceBacking.rid === 3 and once with resourceBacking.rid === 4. The latter is presumably a bad resource ID, which causes op_read_all to throw in a manner that's unexpected by the exception handler.

[lucacasonato]

We'll fix this. However, you probably want to use child.status, not child.output(), because you are consuming both streams manually already?

[cscheid]

Thank you!!

wrt status vs output: Thanks also. I didn't write the original code and was honestly somewhat fumbling around. When this is merged onto Deno I'll try it suggestion, and I hope it gets us unblocked from 1.41.