Re: [whatwg/streams] ReadableStream: releaseLock cannot be called if read() resolves (#1000)

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

In [your `_read()` method](https://github.com/Borewit/readable-web-to-node-stream/blob/891285d073ecb77f8537d02d96feb4ae050b2157/src/index.ts#L35), 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:
```js
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. 😁

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

Received on Friday, 3 May 2019 22:06:09 UTC