ReadableStream: releaseLock cannot be called if read() resolves

[Borewit]

Or maybe I should say, I cannot find any way cancelling a ReadableStream without running into a synchronization issues.

To cancel a ReadableStream the reader has to release the lock on the stream.
Releasing the Reader requires to call releaseLock().

This call will throw an error of there is an outstanding read.

stream = response.body;  // some ReadableStream;

stream.getReader();
this.pendingRead = this.reader.read();

Let's assume the read is still pending here....

At the same time, I would like to get rid of the stream.

Aiming to call releaseLock(), when there is no ongoing read I try to used the promise returned by the read.

await this.pendingRead; // So let's wait for the read to complete, zzz....

// Should be fine now, right?
this.reader.releaseLock(); // Kaboom: Tried to release a reader lock when that reader has pending read() calls un-settled

It looks like, the internal mechanism is resolved after the client side of the promise is resolved.

The issue can be demonstrated on Google Chrome, I am not sure that reflects this reference implementation. I use a Readable stream return by fetch.

[MattiasBuelens]

I don't believe this is a bug in the spec, but rather a bug in your code.

In your _read() method, after the pendingRead promise settles and you've pushed the result onto the Node stream, you delete this.pendingRead. However, if _read() was called again before that promise has settled, then that second call would have replaced this.pendingRead with another promise. This means that the first call deletes the pendingRead too soon: it will clear it before the read promise from the second call has settled. As a result, waitForReadToComplete() incorrectly thinks that there are no pending reads to await anymore, and it tries to unlock the reader too soon.

One way to fix this is by checking if this.pendingRead has changed after you've set it. If it has, then you know _read() has been called again, and that later call will take care of clearing it:

public async _read() {
  // Should start pushing data into the queue
  // Read data from the underlying Web-API-readable-stream
  if (this.released) {
    return;
  }
  let readPromise = this.reader.read();
  this.pendingRead = readPromise;
  try {
    const data = await readPromise;
    if (data.done || this.released) {
      this.push(null);
    } else {
      this.bytesRead += data.value.length;
      this.push(data.value);
    }
  } finally {
    if (readPromise === this.pendingRead) { // <<<
      delete this.pendingRead;
    }
  }
}

If that still doesn't fix it, I could try to dig a bit deeper into your code. If you have a small reproduction case for this issue, that'd be great. 😁

[Borewit]

Thank a lot @MattiasBuelens for your explanation. I think you are absolutely right, it is caused on my end, by a second read. But I think the issue is even simpler to solve. It's a node stream calling Web-API ReadableStream-read. The _read() (Node stream implementation) will not call another _read until data is pushed. The second read is result "in the same call" as the push. To it looks like simply deleting the cached this.promise before pushing data into the Node-readable-queue solves the issue.