Skip to content

Commit

Permalink
stream: fix ReadableStreamReader.releaseLock()
Browse files Browse the repository at this point in the history
Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com
PR-URL: nodejs#44292
Refs: https://streams.spec.whatwg.org/#default-reader-release-lock
Refs: https://streams.spec.whatwg.org/#byob-reader-release-lock
Refs: https://developer.mozilla.org/en-US/docs/Web/API/ReadableStreamDefaultReader/releaseLock
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
daeyeon authored and Fyko committed Sep 15, 2022
1 parent 684ef6c commit b70fce8
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 41 deletions.
42 changes: 32 additions & 10 deletions lib/internal/webstreams/readablestream.js
Original file line number Diff line number Diff line change
Expand Up @@ -809,11 +809,7 @@ class ReadableStreamDefaultReader {
throw new ERR_INVALID_THIS('ReadableStreamDefaultReader');
if (this[kState].stream === undefined)
return;
if (this[kState].readRequests.length) {
throw new ERR_INVALID_STATE.TypeError(
'Cannot release with pending read requests');
}
readableStreamReaderGenericRelease(this);
readableStreamDefaultReaderRelease(this);
}

/**
Expand Down Expand Up @@ -930,11 +926,7 @@ class ReadableStreamBYOBReader {
throw new ERR_INVALID_THIS('ReadableStreamBYOBReader');
if (this[kState].stream === undefined)
return;
if (this[kState].readIntoRequests.length) {
throw new ERR_INVALID_STATE.TypeError(
'Cannot release with pending read requests');
}
readableStreamReaderGenericRelease(this);
readableStreamBYOBReaderRelease(this);
}

/**
Expand Down Expand Up @@ -1730,6 +1722,36 @@ function readableStreamReaderGenericInitialize(reader, stream) {
}
}

function readableStreamDefaultReaderRelease(reader) {
readableStreamReaderGenericRelease(reader);
readableStreamDefaultReaderErrorReadRequests(
reader,
new ERR_INVALID_STATE.TypeError('Releasing reader')
);
}

function readableStreamDefaultReaderErrorReadRequests(reader, e) {
for (let n = 0; n < reader[kState].readRequests.length; ++n) {
reader[kState].readRequests[n][kError](e);
}
reader[kState].readRequests = [];
}

function readableStreamBYOBReaderRelease(reader) {
readableStreamReaderGenericRelease(reader);
readableStreamBYOBReaderErrorReadIntoRequests(
reader,
new ERR_INVALID_STATE.TypeError('Releasing reader')
);
}

function readableStreamBYOBReaderErrorReadIntoRequests(reader, e) {
for (let n = 0; n < reader[kState].readIntoRequests.length; ++n) {
reader[kState].readIntoRequests[n][kError](e);
}
reader[kState].readIntoRequests = [];
}

function readableStreamReaderGenericRelease(reader) {
const {
stream,
Expand Down
18 changes: 4 additions & 14 deletions test/parallel/test-whatwg-readablestream.js
Original file line number Diff line number Diff line change
Expand Up @@ -328,28 +328,18 @@ assert.throws(() => {
const read1 = reader.read();
const read2 = reader.read();

// The stream is empty so the read will never settle.
read1.then(
common.mustNotCall(),
common.mustNotCall()
);

// The stream is empty so the read will never settle.
read2.then(
common.mustNotCall(),
common.mustNotCall()
);
read1.then(common.mustNotCall(), common.mustCall());
read2.then(common.mustNotCall(), common.mustCall());

assert.notStrictEqual(read1, read2);

assert.strictEqual(reader[kState].readRequests.length, 2);

delay().then(common.mustCall());

assert.throws(() => reader.releaseLock(), {
code: 'ERR_INVALID_STATE',
});
assert(stream.locked);
reader.releaseLock();
assert(!stream.locked);
}

{
Expand Down
17 changes: 0 additions & 17 deletions test/wpt/status/streams.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@
"readable-byte-streams/general.any.js": {
"fail": {
"expected": [
"ReadableStream with byte source: releaseLock() on ReadableStreamDefaultReader must reject pending read()",
"ReadableStream with byte source: releaseLock() on ReadableStreamBYOBReader must reject pending read()",
"pull() resolving should not resolve read()",
"ReadableStream with byte source: enqueue() discards auto-allocated BYOB request",
"ReadableStream with byte source: releaseLock() with pending read(view), read(view) on second reader, respond()",
"ReadableStream with byte source: releaseLock() with pending read(view), read(view) on second reader with 1 element Uint16Array, respond(1)",
Expand Down Expand Up @@ -87,20 +84,6 @@
"readable-streams/cross-realm-crash.window.js": {
"skip": "Browser-specific test"
},
"readable-streams/default-reader.any.js": {
"fail": {
"expected": [
"Second reader can read chunks after first reader was released with pending read requests"
]
}
},
"readable-streams/templated.any.js": {
"fail": {
"expected": [
"ReadableStream (empty) reader: releasing the lock should reject all pending read requests"
]
}
},
"transferable/deserialize-error.window.js": {
"skip": "Browser-specific test"
},
Expand Down

0 comments on commit b70fce8

Please sign in to comment.