[whatwg/streams] Readable byte stream must call pull after receiving new chunk (#877)

After a readable byte stream receives a new chunk from the underlying source, the stream must call `pull` (if needed). Otherwise, the stream may get stuck (see test below).

For regular streams, this is already done correctly: `ReadableStreamDefaultController` correctly calls `ReadableStreamDefaultControllerCallPullIfNeeded`. However, byte streams do not call the corresponding `ReadableByteStreamControllerCallPullIfNeeded` in response to `enqueue`, `respond` or `respondWith`.

## Test
The following test (written as a Web platform test) demonstrates the problem. Here, a readable byte stream is constructed that will enqueue a new chunk every time `pull` is called, and closes the stream after 3 chunks. (This test is derived from the existing test _"ReadableStream with byte source: Respond to pull() by enqueue() asynchronously"_, but enqueues its chunks in separate pulls instead.)

This test *times out* with the current reference implementation. The first read resolves, but the other reads remain pending. `pull` is only called 1 time, rather than the expected 4 times (3 times to enqueue a chunk, 1 last time to close the source).

However, if the test is changed to use regular readable streams instead of byte streams, the test passes (see highlighted line).
```
promise_test(async () => {
  let pullCount = 0;

  let byobRequest;
  const desiredSizes = [];

  const stream = new ReadableStream({
    pull(c) {
      byobRequest = c.byobRequest;
      desiredSizes.push(c.desiredSize);

      if (pullCount < 3) {
        c.enqueue(new Uint8Array(1));
      } else {
        c.close();
      }

      ++pullCount;
    },
    type: 'bytes' // <<< removing this line makes the test pass
  }, {
    highWaterMark: 256
  });

  const reader = stream.getReader();

  const p0 = reader.read();
  const p1 = reader.read();
  const p2 = reader.read();

  assert_equals(pullCount, 0, 'No pull as start() just finished and is not yet reflected to the state of the stream');

  return Promise.all([p0, p1, p2]).then(result => {
    assert_equals(pullCount, 4, 'pullCount after completion of all read()s');

    assert_equals(result[0].done, false, 'result[0].done');
    assert_equals(result[0].value.byteLength, 1, 'result[0].value.byteLength');
    assert_equals(result[1].done, false, 'result[1].done');
    assert_equals(result[1].value.byteLength, 1, 'result[1].value.byteLength');
    assert_equals(result[2].done, false, 'result[2].done');
    assert_equals(result[2].value.byteLength, 1, 'result[2].value.byteLength');
    assert_equals(byobRequest, undefined, 'byobRequest should be undefined');
    assert_equals(desiredSizes[0], 256, 'desiredSize on pull should be 256');
    assert_equals(desiredSizes[1], 256, 'desiredSize after 1st enqueue() should be 256');
    assert_equals(desiredSizes[2], 256, 'desiredSize after 2nd enqueue() should be 256');
    assert_equals(desiredSizes[3], 256, 'desiredSize after 3rd enqueue() should be 256');
  });
}, 'ReadableStream with byte source: Respond to multiple pull() by seperate enqueue()');
```

## Proposed solution
I believe the fix should be to add calls to `ReadableByteStreamControllerCallPullIfNeeded` at the end of:
* `ReadableByteStreamControllerEnqueue` (handles `enqueue`)
* `ReadableByteStreamControllerRespondInternal` (handles `respond` and `respondWith`)

-- 
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/877

Received on Tuesday, 6 February 2018 23:35:47 UTC