1752662 - WPT streams/readable-byte-streams/tee.any.js - TypeError: Pending PullInto has detached buffer

Status: RESOLVED FIXED

(Core :: DOM: Streams, defect, P1)

Description

[Kagami Rosylight [:saschanaz] (they/them)]

"ReadableStream teeing with byte source: read from branch2, then read from branch1" test times out and throws an error somehow to the global without rejecting any promise.

Per https://wpt.fyi/results/streams/readable-byte-streams/tee.any.js?label=experimental&label=master&aligned no browser is passing the test, so I wonder who's the culprit here - the test or our impl?

Comment 1

[Tom S [:evilpie]]

Chrome doesn't implement ReadableByteStreamTee, so they use ReadableStreamDefaultTee for everything. In theory there is a reference implementation in JS, I would hope that it passes all tests, but I've never tried it.

Comment 2

[Kagami Rosylight [:saschanaz] (they/them)]

The CI on the spec repo makes sure that the reference implementation passes every test, so it should be us. 👀 (Or maybe the spec, who knows)

Comment 3

[Kagami Rosylight [:saschanaz] (they/them)]

Taking a look... but feel free to add some comment if you already have an idea what's going on.

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

Still investigating... What I understand so far is that the buffer of the PullIntoDescriptor made by ReadableByteStreamControllerPullInto (from reader2.read()) is transferred to another PullIntoDescriptor made during ByteStreamTeePullAlgorithm, and that causes a problem:

[0x0]   xul!mozilla::dom::TransferArrayBuffer   
[0x1]   xul!mozilla::dom::ReadableByteStreamControllerPullInto + 0x1d8   
[0x2]   xul!mozilla::dom::ReadableStreamBYOBReaderRead + 0x18c   
[0x3]   xul!mozilla::dom::PullWithBYOBReader + 0x261   
[0x4]   xul!mozilla::dom::ByteStreamTeePullAlgorithm + 0x1a6   
[0x5]   xul!mozilla::dom::NativeByteStreamTeePullAlgorithm::PullCallback + 0x111   
[0x6]   xul!mozilla::dom::ReadableByteStreamControllerCallPullIfNeeded + 0x12d   
[0x7]   xul!mozilla::dom::ReadableByteStreamControllerPullInto + 0xb01   

Specifically this line passes the view: https://searchfox.org/mozilla-central/rev/4615b544a0f7166233c409c619b426c4025467a7/dom/streams/ReadableStreamTee.cpp#270

Comment 5

[Tom S [:evilpie]]

Nice work pin pointing this line! The fixes is to simply pass the correct argument for aForBranch2.

-    PullWithBYOBReader(aCx, aTeeState, view, false, aRv);
+    PullWithBYOBReader(aCx, aTeeState, view, /* aForBranch2 */ index == 2, aRv);

I think Matthew suggested using an enum index before and this just underlines that.

Comment 6

[Kagami Rosylight [:saschanaz] (they/them)]

OMG C++ 😭

Comment 7

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1752662 - Use enum class TeeBranch instead of boolean/integer r=evilpie

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd7d69e519d9
Use enum class TeeBranch instead of boolean/integer r=evilpie

Comment 9

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/fd7d69e519d9