1750564 - Implement readable byte streams

Status: RESOLVED FIXED

(Core :: DOM: Streams, task, P3)

Description

[Tom S [:evilpie]]

Most of the code for readable byte streams was already implemented by Matthew in bug 1734873, but most of the code is actually not being used yet. Exposing both ReadableByteStreamController and ReadableStreamBYOBReader shows various test failures in the WPT. More concerning is a possible pointer corruption during tracing after mach wpt streams/readable-byte-streams/general.any.html is finished running.

Comment 1

[Tom S [:evilpie]]

Attachment: Bug 1750564 - Implement user-facing readable byte streams. r?mgaudet

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

Hmm. I have yet to spot any memory corruption.

Taking the above attached patch, applying it, then building with the following patch

diff --git a/dom/streams/ReadableByteStreamController.cpp b/dom/streams/ReadableByteStreamController.cpp
--- a/dom/streams/ReadableByteStreamController.cpp
+++ b/dom/streams/ReadableByteStreamController.cpp
@@ -1150,11 +1150,11 @@ void ReadableByteStreamControllerRespond
 
   // Step 2.
   JS::Rooted<JSObject*> buffer(aCx, firstDescriptor->Buffer());
-#ifdef DEBUG
-  bool canTransferBuffer = CanTransferArrayBuffer(aCx, buffer, aRv);
-  MOZ_ASSERT(!aRv.Failed());
-  // MOZ_ASSERT(canTransferBuffer);
-#endif
+  // #ifdef DEBUG
+  //   bool canTransferBuffer = CanTransferArrayBuffer(aCx, buffer, aRv);
+  //   MOZ_ASSERT(!aRv.Failed());
+  //   // MOZ_ASSERT(canTransferBuffer);
+  // #endif
 
   // Step 3.
   ReadableByteStreamControllerInvalidateBYOBRequest(aController);
diff --git a/testing/web-platform/meta/streams/readable-byte-streams/__dir__.ini b/testing/web-platform/meta/streams/readable-byte-streams/__dir__.ini
--- a/testing/web-platform/meta/streams/readable-byte-streams/__dir__.ini
+++ b/testing/web-platform/meta/streams/readable-byte-streams/__dir__.ini
@@ -1,1 +1,1 @@
-disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1454655
+

I get the following errors, repeated across the window and worker types. I ran wpt like this; ./mach wpt --headless testing/web-platform/tests/streams/readable-byte-streams/general.any.js

FAIL ReadableStream with byte source: autoAllocateChunkSize - assert_equals: pull() must have been invoked once expected 1 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:343:18
FAIL ReadableStream with byte source: Mix of auto allocate and BYOB - promise_test: Unhandled rejection with value: object "TypeError: can't access property "view", byobRequest is null"FAIL ReadableStream with byte source: enqueue(), read(view) partially, then read() - assert_equals: value.byteLength expected 8 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:607:18
FAIL ReadableStream with byte source: read(view), then respond() - assert_equals: result.value.byteLength expected 1 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:841:18
FAIL ReadableStream with byte source: read(view), then respondWithNewView() with a transferred ArrayBuffer - promise_test: Unhandled rejection with value: object "TypeError: ReadableStreamBYOBRequest.respondWithNewView: View has zero length in readable stream"FAIL ReadableStream with byte source: respond(3) to read(view) with 2 element Uint16Array enqueues the 1 byte remainder - assert_equals: byteLength expected 2 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:958:18
FAIL ReadableStream with byte source: enqueue(), getReader(), then read(view) - assert_equals: expected 16 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:1001:18
FAIL ReadableStream with byte source: enqueue(), getReader(), then read(view) where view.buffer is not fully covered by view - assert_equals: expected 8 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:1183:18
FAIL ReadableStream with byte source: Multiple enqueue(), getReader(), then read(view) - assert_equals: byteLength expected 24 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:1222:18
FAIL ReadableStream with byte source: enqueue(), getReader(), then read(view) with a bigger view - assert_equals: expected 16 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:1252:18
FAIL ReadableStream with byte source: enqueue(), getReader(), then read(view) with smaller views - assert_equals: expected 8 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:1280:18
FAIL ReadableStream with byte source: enqueue() 1 byte, getReader(), then read(view) with Uint16Array - assert_equals: expected 2 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:1329:18
FAIL ReadableStream with byte source: enqueue() 3 byte, getReader(), then read(view) with 2-element Uint16Array - assert_equals: byteLength expected 2 but got 0
promise<@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:1386:20
FAIL ReadableStream with byte source: read(view), then respond() and close() in pull() - assert_equals: expected 16 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:1533:18
FAIL ReadableStream with byte source: read(view) with Uint32Array, then fill it by multiple respond() calls - assert_equals: result.value.byteLength expected 4 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:1589:18
FAIL ReadableStream with byte source: read(view) with Uint32Array, then fill it by multiple enqueue() calls - assert_equals: result.value.byteLength expected 4 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:1642:18
FAIL ReadableStream with byte source: Multiple read(view), big enqueue() - assert_equals: 1st read: byteLength expected 16 but got 0
p0<@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:1777:18
FAIL ReadableStream with byte source: Multiple read(view) and multiple enqueue() - assert_equals: expected 1024 but got 0
pump/<@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:1812:22
FAIL ReadableStream with byte source: default reader + autoAllocateChunkSize + byobRequest interaction - can't access property "respond", br is null
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:2156:3
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2534:25
promise_test/tests.promise_tests</<@http://web-platform.test:8000/resources/testharness.js:677:36
promise_test/tests.promise_tests<@http://web-platform.test:8000/resources/testharness.js:676:20
FAIL ReadableStream with byte source: autoAllocateChunkSize cannot be 0 - assert_throws_js: controller cannot be setup with autoAllocateChunkSize = 0 function "() => new ReadableStream({ autoAllocateChunkSize: 0, type: 'bytes' })" did not throw
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:2162:19
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2534:25
test@http://web-platform.test:8000/resources/testharness.js:572:30
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:2161:5
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.worker.js:9:14
FAIL ReadableStream with byte source: respondWithNewView() with a smaller view - assert_equals: result.value.byteLength expected 1 but got 0
@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:2230:16
FAIL ReadableStream with byte source: respondWithNewView() with a transferred non-zero-length view (in the readable state) - promise_test: Unhandled rejection with value: object "TypeError: ReadableStreamBYOBRequest.respondWithNewView: View has zero length in readable stream"FAIL ReadableStream with byte source: enqueue() discards auto-allocated BYOB request - promise_test: Unhandled rejection with value: object "TypeError: can't access property "view", byobRequest1 is null"ERROR /streams/readable-byte-streams/general.any.sharedworker.html - Unhandled rejection: can't access property "view", byobRequest is null
pull@http://web-platform.test:8000/streams/readable-byte-streams/general.any.js:313:20

Comment 3

[Matthew Gaudet (he/him) [:mgaudet]]

Tom suggested this was GC linked: I managed to get a crash by appending

promise_test(async t => {
  await garbageCollect();
  assert_equals(true, true, "awaited gc")
})

to the end of testing/web-platform/tests/streams/readable-byte-streams/general.any.js

The problem is I missed adding ReadableByteStreamController into the tracing list with mozilla::HoldJSObject/mozilla::DropJSObject -- will open another but for that patch.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6cb5679ade12
Implement user-facing readable byte streams. r=mgaudet,emilio

Comment 5

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/6cb5679ade12