- From: Mattias Buelens <notifications@github.com>
- Date: Tue, 06 Feb 2018 15:35:24 -0800
- To: whatwg/streams <streams@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <whatwg/streams/issues/877@github.com>
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