From bc94c5c0e72e0918c73976625dbf87935baf7fa0 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 10 Oct 2022 07:22:33 -0700 Subject: [PATCH 1/6] Revert "Revert "Streams tee refactoring: Documenting the roadmap of the chaos to come."" This reverts commit 0b4717f96ce3ea931f3b08f84c70971e87c346c1. --- src/workerd/api/streams/standard.c++ | 216 ++++++++++++++++++++++++++- src/workerd/api/streams/standard.h | 46 ++++++ 2 files changed, 260 insertions(+), 2 deletions(-) diff --git a/src/workerd/api/streams/standard.c++ b/src/workerd/api/streams/standard.c++ index 60fc2216bb2..d9d382e6e17 100644 --- a/src/workerd/api/streams/standard.c++ +++ b/src/workerd/api/streams/standard.c++ @@ -53,6 +53,8 @@ kj::Maybe getChunkSize( return 1; } +// ====================================================================================== + template bool ReadableLockImpl::lockReader( jsg::Lock& js, @@ -160,6 +162,9 @@ void ReadableLockImpl::PipeLocked::visitForGc(jsg::GcVisitor &visito template kj::Maybe ReadableLockImpl::tryTeeLock( Controller& self) { +// TODO(stream-tee): There will be no more tee controller and we have to rethink tee lock. +// Essentially, tee lock will mean the stream is effectively closed and no longer used because +// the underlying controller references have been passed on to the branches. if (isLockedToReader()) { return nullptr; } @@ -167,6 +172,8 @@ kj::Maybe ReadableLockImpl return state.template get(); } +// TODO(stream-tee): Everything relating to TeeLocked here goes away. + template void ReadableLockImpl::TeeLocked::addBranch(Branch* branch) { KJ_ASSERT(branches.find(BranchPtr(branch)) == nullptr, @@ -257,6 +264,8 @@ void ReadableLockImpl::TeeLocked::visitForGc(jsg::GcVisitor &visitor visitor.visit(maybePulling); } +// ====================================================================================== + template bool WritableLockImpl::isLockedToWriter() const { return !state.template is(); @@ -391,6 +400,8 @@ kj::Maybe> WritableLockImpl::PipeLocked::checkSig return nullptr; } +// ====================================================================================== + template jsg::Promise ReadableImpl::cancel( jsg::Lock& js, @@ -398,6 +409,10 @@ jsg::Promise ReadableImpl::cancel( v8::Local reason) { KJ_ASSERT(state.template is()); KJ_IF_MAYBE(pendingCancel, maybePendingCancel) { + +// TODO(stream-tee): This should only be called if the last consumer known to the queue +// is canceling. + // If we're already waiting for cancel to complete, just return the // already existing pending promise. return pendingCancel->promise.whenResolved(); @@ -420,6 +435,8 @@ bool ReadableImpl::canCloseOrEnqueue() { template ReadRequest ReadableImpl::dequeueReadRequest() { +// TODO(stream-tee): This goes away. The Queue::Consumer is responsible for tracking +// read requests. KJ_ASSERT(!readRequests.empty()); auto request = kj::mv(readRequests.front()); readRequests.pop_front(); @@ -464,6 +481,10 @@ void ReadableImpl::doCancel( template void ReadableImpl::doClose(jsg::Lock& js) { +// TODO(stream-tee): Closing and resetting the queue also needs to notify all of the +// still-attached consumers that they should close and detach immediately. All still +// pending reads should be closed out (if they are partially fulfilled) or canceled, +// and the references holding this controller should be cleared. if (!state.template is()) { return; } @@ -485,6 +506,9 @@ void ReadableImpl::doClose(jsg::Lock& js) { template void ReadableImpl::doError(jsg::Lock& js, v8::Local reason) { +// TODO(stream-tee): Error the queue, notify the consumers of the error. All still +// pending reads should be rejected and all still connected consumers should detach, +// keeping record of the error. All references holding this controller should be cleared. if (!state.template is()) { return; } @@ -508,6 +532,7 @@ void ReadableImpl::doError(jsg::Lock& js, v8::Local reason) { template kj::Maybe ReadableImpl::getDesiredSize() { +// TODO(stream-tee): Reimplemented by the queue KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return 0; @@ -530,6 +555,8 @@ bool ReadableImpl::shouldCallPull() { if (!started) { return false; } +// TODO(stream-tee): This will need to be reimplemented to see if any of the known +// consumers want data. if (getOwner().isLocked() && readRequests.size() > 0) { return true; } @@ -577,6 +604,7 @@ template void ReadableImpl::resolveReadRequest( ReadResult result, kj::Maybe maybeRequest) { +// TODO(stream-tee): This goes away because consumers become responsible for resolving reads. if (maybeRequest != nullptr) { maybeResolvePromise(maybeRequest, kj::mv(result)); return; @@ -592,6 +620,7 @@ void ReadableImpl::setup( StreamQueuingStrategy queuingStrategy) { bool isBytes = underlyingSource.type.map([](auto& s) { return s == "bytes"; }).orDefault(false); +// TODO(stream-tee): the highWaterMark is handled by the queue impl highWaterMark = queuingStrategy.highWaterMark.orDefault(isBytes ? 0 : 1); auto startAlgorithm = kj::mv(underlyingSource.start); @@ -631,6 +660,8 @@ void ReadableImpl::visitForGc(jsg::GcVisitor& visitor) { visitor.visitAll(readRequests); } +// ====================================================================================== + template WritableImpl::WritableImpl(WriterOwner& owner) : owner(owner), @@ -1088,6 +1119,9 @@ void WritableImpl::visitForGc(jsg::GcVisitor &visitor) { ReadableStreamDefaultController::ReadableStreamDefaultController(ReaderOwner& owner) : impl(owner) {} +// TODO(stream-tee): The controller will no longer have any notion of a single owner. As long +// as there are consumers known to the queue, the controller will be kept alive. State changes +// are communicated via the consumer. void ReadableStreamDefaultController::setOwner(kj::Maybe owner) { impl.setOwner(owner); @@ -1096,10 +1130,18 @@ void ReadableStreamDefaultController::setOwner(kj::Maybe owner) { jsg::Promise ReadableStreamDefaultController::cancel( jsg::Lock& js, jsg::Optional> maybeReason) { +// TODO(stream-tee): This cancel is triggered by the ReadableStreamJsController via the +// ValueReadable or ByteReadable. Some of the functionality needs to be moved into those. +// For instance, the individual consumer will be responsible for resolving its own +// collection of read requests. The cancel algorithm will only be invoked when the +// last consumer known to the queue is canceling. return impl.cancel(js, JSG_THIS, maybeReason.orDefault(js.v8Undefined())); } void ReadableStreamDefaultController::close(jsg::Lock& js) { +// TODO(stream-tee): This close is triggered by user-call. It must result in the queue +// being closed, which in turn will communicate the close to each of the consumers. +// It will be the responsibility of the consumers to cancel their pending pull intos. JSG_REQUIRE(impl.canCloseOrEnqueue(), TypeError, "This ReadableStreamDefaultController is closed."); @@ -1110,6 +1152,7 @@ void ReadableStreamDefaultController::close(jsg::Lock& js) { } void ReadableStreamDefaultController::doCancel(jsg::Lock& js, v8::Local reason) { +// TODO(stream-tee): Revisit this impl.doCancel(js, JSG_THIS, reason); } @@ -1125,6 +1168,9 @@ void ReadableStreamDefaultController::enqueue( void ReadableStreamDefaultController::doEnqueue( jsg::Lock& js, jsg::Optional> chunk) { +// TODO(stream-tee): Do we need the separate doEnqueue? The implementation here changes a bit. +// the enqueue will push into the ValueQueue and the ValueQueue::Consumers will handle resolving +// the reads. KJ_ASSERT(impl.canCloseOrEnqueue()); auto value = chunk.orDefault(js.v8Undefined()); @@ -1150,6 +1196,8 @@ void ReadableStreamDefaultController::doEnqueue( } void ReadableStreamDefaultController::error(jsg::Lock& js, v8::Local reason) { +// TODO(stream-tee): Called by user-call. We need to notify the queue of the error and notify +// all of the consumers about the error. if (impl.state.is()) { impl.doError(js, reason); } @@ -1160,10 +1208,17 @@ kj::Maybe ReadableStreamDefaultController::getDesiredSize() { } bool ReadableStreamDefaultController::hasPendingReadRequests() { +// TODO(stream-tee): Needs to be modified such that if any consumer has pending reads, this +// returns true. return !impl.readRequests.empty(); } void ReadableStreamDefaultController::pull(jsg::Lock& js, ReadRequest readRequest) { +// TODO(stream-tee): Needs to be reimplemented in terms of the consumer. Essentially, the consumer +// receives the read request. If the consumer doesn't have the data in it's own queue to serve +// the request, it will ask the queue to get it. If there's no data in the queue, then we need +// to pull from the underlying source. + // This should only be called if the stream is readable KJ_ASSERT(impl.state.is()); if (!impl.queue.empty()) { @@ -1208,6 +1263,11 @@ void ReadableStreamDefaultController::setup( impl.setup(js, JSG_THIS, kj::mv(underlyingSource), kj::mv(queuingStrategy)); } +// ====================================================================================== + +// TODO(stream-tee): The ReadableStreamBYOBRequest needs to be modified to operate in +// terms of the ByteQueue::Consumer model. + ReadableStreamBYOBRequest::Impl::Impl( jsg::V8Ref view, jsg::Ref controller, @@ -1301,8 +1361,14 @@ void ReadableStreamBYOBRequest::respondWithNewView(jsg::Lock& js, jsg::BufferSou impl.controller->respondInternal(js, impl.controller->updatePullInto(js, kj::mv(view))); } +// ====================================================================================== + ReadableByteStreamController::ReadableByteStreamController(ReaderOwner& owner) - : impl(owner) {} + : impl(owner) { +// TODO(stream-tee): The notion of a single owner for the controller goes away. The controller +// will have one or more consumers. As long as there are consumers, the controller will be +// available. +} kj::Maybe ReadableByteStreamController::getDesiredSize() { return impl.getDesiredSize(); @@ -1311,6 +1377,11 @@ kj::Maybe ReadableByteStreamController::getDesiredSize() { jsg::Promise ReadableByteStreamController::cancel( jsg::Lock& js, jsg::Optional> maybeReason) { +// TODO(stream-tee): This cancel is triggered by the ReadableStreamJsController via the +// ValueReadable or ByteReadable. Some of the functionality needs to be moved into those. +// For instance, the individual consumer will be responsible for resolving its own +// collection of read requests. The cancel algorithm will only be invoked when the +// last consumer known to the queue is canceling. pendingPullIntos.clear(); while (!impl.readRequests.empty()) { impl.dequeueReadRequest().resolve(ReadResult { .done = true }); @@ -1319,6 +1390,9 @@ jsg::Promise ReadableByteStreamController::cancel( } void ReadableByteStreamController::close(jsg::Lock& js) { +// TODO(stream-tee): This close is triggered by user-call. It must result in the queue +// being closed, which in turn will communicate the close to each of the consumers. +// It will be the responsibility of the consumers to cancel their pending pull intos. JSG_REQUIRE(impl.canCloseOrEnqueue(), TypeError, "This ReadableByteStreamController is closed."); if (!impl.queue.empty()) { @@ -1338,6 +1412,7 @@ void ReadableByteStreamController::close(jsg::Lock& js) { } void ReadableByteStreamController::commitPullInto(jsg::Lock& js, PendingPullInto pullInto) { +// TODO(stream-tee): Responsibility here moves into the ByteQueue::Consumer implementation. bool done = false; if (impl.state.is()) { KJ_ASSERT(pullInto.filled == 0); @@ -1353,6 +1428,7 @@ void ReadableByteStreamController::commitPullInto(jsg::Lock& js, PendingPullInto ReadableByteStreamController::PendingPullInto ReadableByteStreamController::dequeuePendingPullInto() { +// TODO(stream-tee): Responsibility here moves into the ByteQueue::Consumer implementation. KJ_ASSERT(!pendingPullIntos.empty()); auto pullInto = kj::mv(pendingPullIntos.front()); pendingPullIntos.pop_front(); @@ -1360,10 +1436,14 @@ ReadableByteStreamController::dequeuePendingPullInto() { } void ReadableByteStreamController::doCancel(jsg::Lock& js, v8::Local reason) { +// TODO(stream-tee): Re-evaluate... impl.doCancel(js, JSG_THIS, reason); } void ReadableByteStreamController::enqueue(jsg::Lock& js, jsg::BufferSource chunk) { +// TODO(stream-tee): This is called by user-call. It must be modified to push data into the queue, +// which will trigger the cascade of that data into the consumers, which will handle the majority +// of the handling here. JSG_REQUIRE(chunk.size() > 0, TypeError, "Cannot enqueue a zero-length ArrayBuffer."); JSG_REQUIRE(chunk.canDetach(js), TypeError, "The provided ArrayBuffer must be detachable."); @@ -1402,12 +1482,15 @@ void ReadableByteStreamController::enqueue(jsg::Lock& js, jsg::BufferSource chun } void ReadableByteStreamController::error(jsg::Lock& js, v8::Local reason) { +// TODO(stream-tee): This is called by the user-call. It must error the queue and communicate to +// each of the consumers that the readable stream is immediately shutting down because of error. if (impl.state.is()) { impl.doError(js, reason); } } bool ReadableByteStreamController::fillPullInto(PendingPullInto& pullInto) { +// TODO(stream-tee): Responsibility here moves into the ByteQueue::Consumer implementation. auto elementSize = pullInto.store.getElementSize(); auto currentAlignedBytes = pullInto.filled - (pullInto.filled % elementSize); auto maxBytesToCopy = kj::min(impl.queue.size(), pullInto.store.size() - pullInto.filled); @@ -1452,6 +1535,8 @@ bool ReadableByteStreamController::fillPullInto(PendingPullInto& pullInto) { kj::Maybe> ReadableByteStreamController::getByobRequest( jsg::Lock& js) { +// TODO(stream-tee): The ReadableStreamBYOBRequest mechanism needs to be reworked around +// ByteQueue::Consumer's similar concept. JSG_REQUIRE(impl.state.is(), TypeError, "This ReadableByteStreamController has been closed."); @@ -1469,6 +1554,9 @@ kj::Maybe> ReadableByteStreamController::get } bool ReadableByteStreamController::hasPendingReadRequests() { +// TODO(stream-tee): Needs to be modified such that if any known consumer has a pending +// read request this returns true. The controller itself will no longer track read requests +// itself. return !impl.readRequests.empty(); } @@ -1477,6 +1565,7 @@ bool ReadableByteStreamController::isReadable() const { } void ReadableByteStreamController::pullIntoUsingQueue(jsg::Lock& js) { +// TODO(stream-tee): Responsibility here moves into the ByteQueue::Consumer implementation. KJ_ASSERT(!impl.closeRequested); while (!pendingPullIntos.empty() && !impl.queue.empty()) { auto& pullInto = pendingPullIntos.front(); @@ -1487,6 +1576,10 @@ void ReadableByteStreamController::pullIntoUsingQueue(jsg::Lock& js) { } void ReadableByteStreamController::pull(jsg::Lock& js, ReadRequest readRequest) { +// TODO(stream-tee): Triggers the pull algorithm to be called on this controller. +// We will need to rework the impl.queue.empty() piece below. Is it even still +// relevant with the new queue model? + // This should only ever be called if the stream is readable KJ_ASSERT(impl.state.is()); if (!impl.queue.empty()) { @@ -1516,6 +1609,10 @@ void ReadableByteStreamController::pull(jsg::Lock& js, ReadRequest readRequest) } void ReadableByteStreamController::queueDrain(jsg::Lock& js) { +// TODO(stream-tee): Need to re-evaluate this. Essentially, this is a graceful +// close. If close has been requested and the queue is empty, we signal to all +// consumers that we're done and they should disconnect, otherwise we try +// pulling more data. if (impl.queue.size() == 0 && impl.closeRequested) { return impl.doClose(js); } @@ -1525,7 +1622,8 @@ void ReadableByteStreamController::queueDrain(jsg::Lock& js) { jsg::Promise ReadableByteStreamController::read( jsg::Lock& js, kj::Maybe maybeByobOptions) { - +// TODO(stream-tee): This likely goes away. It will be up to the ByteQueue::Consumer +// to handle this functionality. if (impl.state.is()) { KJ_IF_MAYBE(byobOptions, maybeByobOptions) { // We're going to return an empty ArrayBufferView using the same underlying buffer but with @@ -1603,6 +1701,7 @@ jsg::Promise ReadableByteStreamController::read( } void ReadableByteStreamController::respondInternal(jsg::Lock& js, size_t bytesWritten) { +// TODO(stream-tee): This likely goes away and is handled by the ByteQueue::Consumer. auto& pullInto = pendingPullIntos.front(); KJ_DEFER(KJ_IF_MAYBE(byobRequest, maybeByobRequest) { (*byobRequest)->invalidate(js); @@ -1660,6 +1759,7 @@ void ReadableByteStreamController::setup( } size_t ReadableByteStreamController::updatePullInto(jsg::Lock& js, jsg::BufferSource view) { +// TODO(stream-tee): This likely goes away and is handled by the ByteQueue::Consumer. auto& pullInto = pendingPullIntos.front(); auto byteLength = view.size(); JSG_REQUIRE(view.canDetach(js), TypeError, @@ -1677,6 +1777,10 @@ size_t ReadableByteStreamController::updatePullInto(jsg::Lock& js, jsg::BufferSo return byteLength; } +// ====================================================================================== + +// TODO(stream-tee): Everything JsTeeController goes away + ReadableStreamJsTeeController::Attached::Attached( jsg::Ref ref, TeeController& controller) @@ -2077,6 +2181,8 @@ kj::Maybe ReadableStreamJsTeeControll return lock.tryPipeLock(*this, kj::mv(destination)); } +// ====================================================================================== + ReadableStreamJsController::ReadableStreamJsController() {} ReadableStreamJsController::ReadableStreamJsController(StreamStates::Closed closed) @@ -2104,9 +2210,19 @@ jsg::Promise ReadableStreamJsController::cancel( return js.rejectedPromise(errored.addRef(js)); } KJ_CASE_ONEOF(controller, ByobController) { + +// TODO(stream-tee): +// KJ_CASE_ONEOF(byteReadable, kj::Own) { +// return byteReadable->cancel(js, kj::mv(maybeReason)); + return controller->cancel(js, reason.getHandle(js)); } KJ_CASE_ONEOF(controller, DefaultController) { + +// TODO(stream-tee): +// KJ_CASE_ONEOF(valueReadable, kj::Own) { +// return valueReadable->cancel(js, kj::mv(maybeReason)); + return controller->cancel(js, reason.getHandle(js)); } } @@ -2115,6 +2231,9 @@ jsg::Promise ReadableStreamJsController::cancel( } void ReadableStreamJsController::doCancel(jsg::Lock& js, v8::Local reason) { +// TODO(stream-tee): Likely not necessary to implement this with the new model? +// This is intended to allow completion of canceling the controller which will be +// done via the ByteReadable or ValueReadable when those are canceled. KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return; @@ -2133,6 +2252,11 @@ void ReadableStreamJsController::doCancel(jsg::Lock& js, v8::Local re } void ReadableStreamJsController::detachFromController() { +// TODO(stream-tee): With the new model, the ValueReadable or ByteReadable struct +// will become the new owner of the relationship with the underlying controller. +// There will no longer be a single owner known to the controller, only multiple +// consumers. This function likely goes away entirely in favor of some mechanism +// on the ValueReadable/ByteReadable. KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) {} KJ_CASE_ONEOF(errored, StreamStates::Errored) {} @@ -2146,6 +2270,15 @@ void ReadableStreamJsController::detachFromController() { } void ReadableStreamJsController::doClose() { +// TODO(stream-tee): doClose() finalizes the closed state of this ReadableStream. +// The connection to the underlying controller is released with no further action. +// Importantly, this method is triggered by the underlying controller as a result +// of that controller closing or being canceled. We detach ourselves from the +// underlying controller by releasing the ValueReadable or ByteReadable in the +// state and changing that to closed. We also clean up other state here. +// Since the underlying controller will no longer have a single owner, we will +// need to modify things such this signal is triggered by the ValueReadable or +// ByteReadable. detachFromController(); state.init(); @@ -2165,6 +2298,7 @@ void ReadableStreamJsController::doClose() { } void ReadableStreamJsController::controllerClose(jsg::Lock& js) { +// TODO(stream-tee): This is obsolete and no longer needed. KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return; } KJ_CASE_ONEOF(errored, StreamStates::Errored) { return; } @@ -2181,6 +2315,7 @@ void ReadableStreamJsController::controllerClose(jsg::Lock& js) { void ReadableStreamJsController::controllerError( jsg::Lock& js, v8::Local reason) { +// TODO(stream-tee): This is obsolete and no longer needed. KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return; } KJ_CASE_ONEOF(errored, StreamStates::Errored) { return; } @@ -2195,6 +2330,15 @@ void ReadableStreamJsController::controllerError( } void ReadableStreamJsController::doError(jsg::Lock& js, v8::Local reason) { +// TODO(stream-tee): As with doClose(), doError() finalizes the error state of this +// ReadableStream. The connection to the underlying controller is released with no +// further action. This method is triggered by the underlying controller as a result +// of that controller erroring or being canceled. We detach ourselves from the +// underlying controller by releasing the ValueReadable or ByteReadable in the state +// and changing that to errored. We also clean up other state here. +// Since the underlying controller will no longer have a single owner, we will +// need to modify things such that this signal is triggered by the ValueReadable or +// ByteReadable. detachFromController(); state.init(js.v8Ref(reason)); @@ -2218,9 +2362,19 @@ bool ReadableStreamJsController::hasPendingReadRequests() { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return false; } KJ_CASE_ONEOF(errored, StreamStates::Errored) { return false; } KJ_CASE_ONEOF(controller, DefaultController) { + +// TODO(stream-tee): +// KJ_CASE_ONEOF(valueReadable, kj::Own) { +// return valueReadable->consumer->hasPendingReadRequests(); + return controller->hasPendingReadRequests(); } KJ_CASE_ONEOF(controller, ByobController) { + +// TODO(stream-tee): +// KJ_CASE_ONEOF(byteReadable, kj::Own) { +// return byteReadable->consumer->hasPendingReadRequests(); + return controller->hasPendingReadRequests(); } } @@ -2228,6 +2382,8 @@ bool ReadableStreamJsController::hasPendingReadRequests() { } bool ReadableStreamJsController::isByteOriented() const { +// TODO(stream-tee): +// return state.is>(); return state.is(); } @@ -2311,6 +2467,10 @@ kj::Maybe> ReadableStreamJsController::read( return js.rejectedPromise(errored.addRef(js)); } KJ_CASE_ONEOF(controller, DefaultController) { + +// TODO(stream-tee): Read for both ValueReadable and ByteReadable must be updated +// in terms of, e.g. valueReadable->consumer->read(...) + // The ReadableStreamDefaultController does not support ByobOptions. // It should never happen, but let's make sure. KJ_ASSERT(maybeByobOptions == nullptr); @@ -2343,6 +2503,13 @@ ReadableStreamJsController::removeSource(jsg::Lock& js) { kj::throwFatalException(js.exceptionToKj(errored.addRef(js))); } KJ_CASE_ONEOF(controller, ByobController) { + +// TODO(stream-tee): Removing the source here becomes as simple as transferring the +// ValueReadable and ByteReadable from the ReadableStreamJsController to the +// ReadableStreamJsSource that we're creating. All of the relevant state transfers +// with it. We must, however, ensure that there is still a way for that to communicate +// state changes (e.g. closed, errored) with whichever thing is currently owning it. + KJ_DEFER(state.init()); return kj::refcounted(kj::mv(controller)); } @@ -2355,6 +2522,9 @@ ReadableStreamJsController::removeSource(jsg::Lock& js) { } ReadableStreamController::Tee ReadableStreamJsController::tee(jsg::Lock& js) { +// TODO(stream-tee): Here, rather than creating new ReadableStreamJsTeeController-backed things, +// we are going to just create new ReadableStreamJsController things that have their own +// clones of this streams ValueReadable or ByteReadable. KJ_IF_MAYBE(teeController, lock.tryTeeLock(*this)) { disturbed = true; @@ -2397,6 +2567,9 @@ void ReadableStreamJsController::setup( maybeTransformer = kj::mv(underlyingSource.maybeTransformer); +// TODO(stream-tee): Here, we wrap create the underlying controllers but we need to +// create the ValueReadable or ByteReadable and use that for the state instead. + if (type == "bytes") { state = jsg::alloc(*this); state.get()->setup( @@ -2427,9 +2600,19 @@ void ReadableStreamJsController::visitForGc(jsg::GcVisitor& visitor) { visitor.visit(error); } KJ_CASE_ONEOF(controller, DefaultController) { + +// TODO(stream-tee): +// KJ_CASE_ONEOF(valueReadable, kj::Own) { +// visitor.visit(*valueReadable); + visitor.visit(controller); } KJ_CASE_ONEOF(controller, ByobController) { + +// TODO(stream-tee): +// KJ_CASE_ONEOF(byteReadable, kj::Own) { +// visitor.visit(*byteReadable); + visitor.visit(controller); } } @@ -2437,6 +2620,10 @@ void ReadableStreamJsController::visitForGc(jsg::GcVisitor& visitor) { }; kj::Maybe ReadableStreamJsController::getDesiredSize() { +// TODO(stream-tee): This is used by the TransformStream implementation. This needs to be +// implemented in terms of the underlying controllers backpressure, such that even if there +// is no data waiting in this particular stream's buffer, backpressure is still being signalled +// correctly through every consumer branch. KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return nullptr; } KJ_CASE_ONEOF(errored, StreamStates::Errored) { return nullptr; } @@ -2457,6 +2644,8 @@ kj::Maybe> ReadableStreamJsController::isErrored(jsg::Lock& } bool ReadableStreamJsController::canCloseOrEnqueue() { +// TODO(stream-tee): This is used by the TransformStream implementation. The implementation +// here won't need to change much. KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return false; } KJ_CASE_ONEOF(errored, StreamStates::Errored) { return false; } @@ -2471,6 +2660,8 @@ bool ReadableStreamJsController::canCloseOrEnqueue() { } bool ReadableStreamJsController::hasBackpressure() { +// TODO(stream-tee): This is used by the TransformStream implementation. Need to determine +// however if we can get rid of this and just use negative or zero desiredSize. KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return false; } KJ_CASE_ONEOF(errored, StreamStates::Errored) { return false; } @@ -2487,16 +2678,24 @@ bool ReadableStreamJsController::hasBackpressure() { void ReadableStreamJsController::defaultControllerEnqueue( jsg::Lock& js, v8::Local chunk) { +// TODO(stream-tee): Is this necessary still? auto& controller = KJ_ASSERT_NONNULL(state.tryGet(), "defaultControllerEnqueue() can only be called with a ReadableStreamDefaultController"); controller->doEnqueue(js, chunk); } +// ====================================================================================== + void ReadableStreamJsSource::cancel(kj::Exception reason) { const auto doCancel = [this](auto& controller, auto reason) { JSG_REQUIRE(!canceling, TypeError, "The stream has already been canceled."); canceling = true; +// TODO(stream-tee): The change here will echo what is happening over in JsController. +// Specifically, the JsSource will have either a ValueReadable or ByteReadable that +// is one of several owning a reference to the underlying controller. When the last +// one canceled is the one that actually triggers the underlying controller cancel. + ioContext.addTask(ioContext.run([this, &controller, reason = kj::mv(reason)] (Worker::Lock& lock) mutable -> kj::Promise { detachFromController(); @@ -2522,6 +2721,8 @@ void ReadableStreamJsSource::cancel(kj::Exception reason) { } void ReadableStreamJsSource::detachFromController() { +// TODO(stream-tee): Since it will be the ValueReadable or ByteReadable that deals +// with this now, hopefully we can remove this. KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) {} KJ_CASE_ONEOF(errored, kj::Exception) {} @@ -2535,11 +2736,13 @@ void ReadableStreamJsSource::detachFromController() { } void ReadableStreamJsSource::doClose() { +// TODO(stream-tee): Similar change here as JsController::doClose detachFromController(); state.init(); } void ReadableStreamJsSource::doError(jsg::Lock& js, v8::Local reason) { +// TODO(stream-tee): Similar change here as JsController::doError detachFromController(); state.init(js.exceptionToKj(js.v8Ref(reason))); } @@ -2548,6 +2751,9 @@ bool ReadableStreamJsSource::isLocked() const { return true; } bool ReadableStreamJsSource::isLockedReaderByteOriented() { return true; } +// TODO(stream-tee): The read mechanisms below must be updated in terms of using +// the ValueReadable or ByteReadable. + jsg::Promise ReadableStreamJsSource::readFromByobController( jsg::Lock& js, void* buffer, @@ -2862,6 +3068,10 @@ jsg::Promise ReadableStreamJsSource::pipeLoop( }); } +// ====================================================================================== + +// TODO(stream-tee): Everything ReadableStreamJsTeeSource goes away. + void ReadableStreamJsTeeSource::cancel(kj::Exception reason) { KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return; } @@ -3134,6 +3344,8 @@ jsg::Promise ReadableStreamJsTeeSource::pipeLoop( }); } +// ====================================================================================== + WritableStreamDefaultController::WritableStreamDefaultController(WriterOwner& owner) : impl(owner) {} diff --git a/src/workerd/api/streams/standard.h b/src/workerd/api/streams/standard.h index a998eec3c9a..61a9cc86536 100644 --- a/src/workerd/api/streams/standard.h +++ b/src/workerd/api/streams/standard.h @@ -6,6 +6,7 @@ #include "common.h" #include "internal.h" +#include "queue.h" #include #include @@ -1278,6 +1279,51 @@ class ReadableStreamJsController: public ReadableStreamController, } private: + struct ValueReadable final { + DefaultController controller; + kj::Own consumer; + + ValueReadable(DefaultController controller); + KJ_DISALLOW_COPY(ValueReadable); + + kj::Own clone(jsg::Lock& js); + // A single ReadableStreamDefaultController can have multiple consumers. + // When the ValueReadable constructor is used, the new consumer is added + // and starts to receive new data that becomes enqueued. When clone + // is used, any state currently held by this consumer is copied to the + // new consumer. + + jsg::Promise cancel(jsg::Lock& js, jsg::Optional> maybeReason); + // When a ReadableStream is canceled, the expected behavior is that the underlying + // controller is notified and the cancel algorithm on the underlying source is + // called. When there are multiple ReadableStreams sharing consumption of a + // controller, however, it should act as a shared pointer of sorts, canceling + // the underlying controller only when the last reader is canceled. + }; + + struct ByteReadable final { + ByobController controller; + kj::Own consumer; + + ByteReadable(DefaultController controller); + KJ_DISALLOW_COPY(ByteReadable); + + kj::Own clone(jsg::Lock& js); + // A single ReadableByteStreamController can have multiple consumers. + // When the ByteReadable constructor is used, the new consumer is added + // and starts to receive new data that becomes enqueued. When clone + // is used, any state currently held by this consumer is copied to the + // new consumer. + + jsg::Promise cancel(jsg::Lock& js, jsg::Optional> maybeReason); + // When a ReadableStream is canceled, the expected behavior is that the underlying + // controller is notified and the cancel algorithm on the underlying source is + // called. When there are multiple ReadableStreams sharing consumption of a + // controller, however, it should act as a shared pointer of sorts, canceling + // the underlying controller only when the last reader is canceled. + }; + + bool hasPendingReadRequests(); void detachFromController(); From 2f1caf5212eee29d375f92e65ea07861ff36cc5d Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 10 Oct 2022 07:22:36 -0700 Subject: [PATCH 2/6] Revert "Revert "Update QueueImpl::ConsumerImpl to support state reporting"" This reverts commit beba902e3cbd814545d1efd99547dcb29d72574a. --- src/workerd/api/streams/queue.c++ | 26 ++++++--- src/workerd/api/streams/queue.h | 39 ++++++++++--- src/workerd/api/streams/standard.h | 89 +++++++++++++++--------------- 3 files changed, 94 insertions(+), 60 deletions(-) diff --git a/src/workerd/api/streams/queue.c++ b/src/workerd/api/streams/queue.c++ index b740bec2dd0..a189ce79568 100644 --- a/src/workerd/api/streams/queue.c++ +++ b/src/workerd/api/streams/queue.c++ @@ -60,9 +60,13 @@ ValueQueue::QueueEntry ValueQueue::QueueEntry::clone() { #pragma region ValueQueue::Consumer -ValueQueue::Consumer::Consumer(ValueQueue& queue) : impl(queue.impl) {} +ValueQueue::Consumer::Consumer( + ValueQueue& queue, + ConsumerImpl::StateListener* stateListener) : impl(queue.impl, stateListener) {} -ValueQueue::Consumer::Consumer(QueueImpl& impl) : impl(impl) {} +ValueQueue::Consumer::Consumer( + QueueImpl& impl, + ConsumerImpl::StateListener* stateListener) : impl(impl, stateListener) {} void ValueQueue::Consumer::close(jsg::Lock& js) { impl.close(js); }; @@ -84,8 +88,10 @@ void ValueQueue::Consumer::reset() { impl.reset(); }; size_t ValueQueue::Consumer::size() { return impl.size(); } -kj::Own ValueQueue::Consumer::clone(jsg::Lock& js) { - auto consumer = kj::heap(impl.queue); +kj::Own ValueQueue::Consumer::clone( + jsg::Lock& js, + ConsumerImpl::StateListener* stateListener) { + auto consumer = kj::heap(impl.queue, stateListener); impl.cloneTo(js, consumer->impl); return kj::mv(consumer); } @@ -234,9 +240,11 @@ ByteQueue::QueueEntry ByteQueue::QueueEntry::clone() { #pragma region ByteQueue::Consumer -ByteQueue::Consumer::Consumer(ByteQueue& queue) : impl(queue.impl) {} +ByteQueue::Consumer::Consumer(ByteQueue& queue, ConsumerImpl::StateListener* stateListener) + : impl(queue.impl, stateListener) {} -ByteQueue::Consumer::Consumer(QueueImpl& impl) : impl(impl) {} +ByteQueue::Consumer::Consumer(QueueImpl& impl, ConsumerImpl::StateListener* stateListener) + : impl(impl, stateListener) {} void ByteQueue::Consumer::close(jsg::Lock& js) { impl.close(js); } @@ -258,8 +266,10 @@ void ByteQueue::Consumer::reset() { impl.reset(); } size_t ByteQueue::Consumer::size() const { return impl.size(); } -kj::Own ByteQueue::Consumer::clone(jsg::Lock& js) { - auto consumer = kj::heap(impl.queue); +kj::Own ByteQueue::Consumer::clone( + jsg::Lock& js, + ConsumerImpl::StateListener* stateListener) { + auto consumer = kj::heap(impl.queue, stateListener); impl.cloneTo(js, consumer->impl); return kj::mv(consumer); } diff --git a/src/workerd/api/streams/queue.h b/src/workerd/api/streams/queue.h index 222d6c31624..14cdeeeba2e 100644 --- a/src/workerd/api/streams/queue.h +++ b/src/workerd/api/streams/queue.h @@ -281,6 +281,11 @@ template class ConsumerImpl final { // Provides the underlying implementation shared by ByteQueue::Consumer and ValueQueue::Consumer public: + struct StateListener { + virtual void onConsumerClose() = 0; + virtual void onConsumerError(jsg::Value reason) = 0; + }; + using QueueImpl = QueueImpl; struct UpdateBackpressureScope final { @@ -299,7 +304,8 @@ class ConsumerImpl final { using Entry = typename Self::Entry; using QueueEntry = typename Self::QueueEntry; - ConsumerImpl(QueueImpl& queue): queue(queue) { + ConsumerImpl(QueueImpl& queue, StateListener* stateListener = nullptr) + : queue(queue), stateListener(stateListener) { queue.addConsumer(*this); } @@ -427,6 +433,16 @@ class ConsumerImpl final { } } + bool hasReadRequests() const { + KJ_SWITCH_ONEOF(state) { + KJ_CASE_ONEOF(closed, Closed) { return false; } + KJ_CASE_ONEOF(errored, Errored) { return false; } + KJ_CASE_ONEOF(ready, Ready) { + return ready.readRequests.empty(); + } + } + } + private: struct Close {}; // A sentinel used in the buffer to signal that close() has been called. @@ -441,6 +457,7 @@ class ConsumerImpl final { QueueImpl& queue; kj::OneOf state = Ready(); + StateListener* stateListener; bool isClosing() { // Closing state is determined by whether there is a Close sentinel that has been @@ -470,6 +487,10 @@ class ConsumerImpl final { request.reject(js, *reason); } state = kj::mv(*reason); + if (stateListener != nullptr) { + stateListener->onConsumerError(reason->addRef(js)); + stateListener = nullptr; + } } else { // Otherwise, if the buffer is empty isClosing() is true, resolve the // remaining read promises with close indicators and update the state @@ -480,6 +501,10 @@ class ConsumerImpl final { request.resolveAsDone(js); } state.template init(); + if (stateListener != nullptr) { + stateListener->onConsumerClose(); + stateListener = nullptr; + } } } } @@ -533,8 +558,8 @@ class ValueQueue final { class Consumer final { public: - Consumer(ValueQueue& queue); - Consumer(QueueImpl& queue); + Consumer(ValueQueue& queue, ConsumerImpl::StateListener* stateListener = nullptr); + Consumer(QueueImpl& queue, ConsumerImpl::StateListener* stateListener = nullptr); Consumer(Consumer&&) = delete; Consumer(Consumer&) = delete; Consumer& operator=(Consumer&&) = delete; @@ -554,7 +579,7 @@ class ValueQueue final { size_t size(); - kj::Own clone(jsg::Lock& js); + kj::Own clone(jsg::Lock& js, ConsumerImpl::StateListener* stateListener = nullptr); private: ConsumerImpl impl; @@ -674,8 +699,8 @@ class ByteQueue final { class Consumer { public: - Consumer(ByteQueue& queue); - Consumer(QueueImpl& queue); + Consumer(ByteQueue& queue, ConsumerImpl::StateListener* stateListener = nullptr); + Consumer(QueueImpl& queue, ConsumerImpl::StateListener* stateListener = nullptr); Consumer(Consumer&&) = delete; Consumer(Consumer&) = delete; Consumer& operator=(Consumer&&) = delete; @@ -695,7 +720,7 @@ class ByteQueue final { size_t size() const; - kj::Own clone(jsg::Lock& js); + kj::Own clone(jsg::Lock& js, ConsumerImpl::StateListener* stateListener = nullptr); private: ConsumerImpl impl; diff --git a/src/workerd/api/streams/standard.h b/src/workerd/api/streams/standard.h index 61a9cc86536..9a9463ba32f 100644 --- a/src/workerd/api/streams/standard.h +++ b/src/workerd/api/streams/standard.h @@ -821,6 +821,50 @@ class WritableImpl { friend Self; }; + +struct ValueReadable final { + DefaultController controller; + kj::Own consumer; + + ValueReadable(DefaultController controller); + KJ_DISALLOW_COPY(ValueReadable); + + kj::Own clone(jsg::Lock& js); + // A single ReadableStreamDefaultController can have multiple consumers. + // When the ValueReadable constructor is used, the new consumer is added + // and starts to receive new data that becomes enqueued. When clone + // is used, any state currently held by this consumer is copied to the + // new consumer. + + jsg::Promise cancel(jsg::Lock& js, jsg::Optional> maybeReason); + // When a ReadableStream is canceled, the expected behavior is that the underlying + // controller is notified and the cancel algorithm on the underlying source is + // called. When there are multiple ReadableStreams sharing consumption of a + // controller, however, it should act as a shared pointer of sorts, canceling + // the underlying controller only when the last reader is canceled. +}; + +struct ByteReadable final { + ByobController controller; + kj::Own consumer; + + ByteReadable(DefaultController controller); + KJ_DISALLOW_COPY(ByteReadable); + + kj::Own clone(jsg::Lock& js); + // A single ReadableByteStreamController can have multiple consumers. + // When the ByteReadable constructor is used, the new consumer is added + // and starts to receive new data that becomes enqueued. When clone + // is used, any state currently held by this consumer is copied to the + // new consumer. + + jsg::Promise cancel(jsg::Lock& js, jsg::Optional> maybeReason); + // When a ReadableStream is canceled, the expected behavior is that the underlying + // controller is notified and the cancel algorithm on the underlying source is + // called. When there are multiple ReadableStreams sharing consumption of a + // controller, however, it should act as a shared pointer of sorts, canceling + // the underlying controller only when the last reader is canceled. +}; } // namespace jscontroller class ReadableStreamDefaultController: public jsg::Object { @@ -1279,51 +1323,6 @@ class ReadableStreamJsController: public ReadableStreamController, } private: - struct ValueReadable final { - DefaultController controller; - kj::Own consumer; - - ValueReadable(DefaultController controller); - KJ_DISALLOW_COPY(ValueReadable); - - kj::Own clone(jsg::Lock& js); - // A single ReadableStreamDefaultController can have multiple consumers. - // When the ValueReadable constructor is used, the new consumer is added - // and starts to receive new data that becomes enqueued. When clone - // is used, any state currently held by this consumer is copied to the - // new consumer. - - jsg::Promise cancel(jsg::Lock& js, jsg::Optional> maybeReason); - // When a ReadableStream is canceled, the expected behavior is that the underlying - // controller is notified and the cancel algorithm on the underlying source is - // called. When there are multiple ReadableStreams sharing consumption of a - // controller, however, it should act as a shared pointer of sorts, canceling - // the underlying controller only when the last reader is canceled. - }; - - struct ByteReadable final { - ByobController controller; - kj::Own consumer; - - ByteReadable(DefaultController controller); - KJ_DISALLOW_COPY(ByteReadable); - - kj::Own clone(jsg::Lock& js); - // A single ReadableByteStreamController can have multiple consumers. - // When the ByteReadable constructor is used, the new consumer is added - // and starts to receive new data that becomes enqueued. When clone - // is used, any state currently held by this consumer is copied to the - // new consumer. - - jsg::Promise cancel(jsg::Lock& js, jsg::Optional> maybeReason); - // When a ReadableStream is canceled, the expected behavior is that the underlying - // controller is notified and the cancel algorithm on the underlying source is - // called. When there are multiple ReadableStreams sharing consumption of a - // controller, however, it should act as a shared pointer of sorts, canceling - // the underlying controller only when the last reader is canceled. - }; - - bool hasPendingReadRequests(); void detachFromController(); From 00b9bb33a09774bc1661ff9db9ad89faabbf20f2 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 10 Oct 2022 07:22:36 -0700 Subject: [PATCH 3/6] Revert "Revert "Refactor JS-backed streams to support multi-consumer tee" This reverts commit 7b9be14194dd736920ca6328cce5235c09768125. --- src/workerd/api/streams/README.md | 34 +- src/workerd/api/streams/internal.c++ | 10 +- src/workerd/api/streams/internal.h | 5 +- src/workerd/api/streams/queue-test.c++ | 8 +- src/workerd/api/streams/queue.c++ | 261 ++- src/workerd/api/streams/queue.h | 218 +- src/workerd/api/streams/readable.c++ | 15 +- src/workerd/api/streams/readable.h | 5 +- src/workerd/api/streams/standard.c++ | 2817 ++++++++---------------- src/workerd/api/streams/standard.h | 871 ++------ src/workerd/jsg/buffersource.c++ | 6 + src/workerd/jsg/buffersource.h | 19 + 12 files changed, 1535 insertions(+), 2734 deletions(-) diff --git a/src/workerd/api/streams/README.md b/src/workerd/api/streams/README.md index 2554e43e4fe..5662464e608 100644 --- a/src/workerd/api/streams/README.md +++ b/src/workerd/api/streams/README.md @@ -240,16 +240,25 @@ completely independent of any of the underlying source algorithms. The `ReadableStream` API has a method `tee()` that will split the flow of data from the `ReadableStream` into two separate `ReadableStream` instances. -What happens here is that ownership of the underlying ***controller*** of the original -`ReadableStream` is passed off to something called the ***Tee Adapter***. The adapter -maintains a collection of ***Tee Branches***. Each branch is a separate `ReadableStream` -maintaining its own queue of available data and pending reads. When the pull algorithm -pushes data into the the underlying ***controller***, the adapter pushes that data to -the internal queues of each of the attached branches. From there, reading from the branch -streams is the same as reading from a regular `ReadableStream` -- that is, when `read()` -is called, if there is data in the internal queue, the read is fulfilled immediately, -otherwise the branch will tell the adapter that it needs data to be provided to fulfill -the pending read. +In the standard definition of the `ReadableStream` API, the `tee()` method creates two +separate `ReadableStream` instances (called "branches") that share a single `Reader` that +consumes the data from the original `ReadableStream` (let's call it the "trunk"). When one +of the two branches uses the shared `Reader` to pull data from the trunk, that data is +used to fulfill the read request from the pulling branch, and a copy of the data is pushed +into a queue in the other branch. That copied data accumulates in memory until something +starts reading from it. + +This spec defined behavior presents a problem for us in that it is possible for one branch +to consume data at a far greater pace than the other, causing the slower branch to accumulate +data in memory without any backpressure controls. + +In our implementation, we have modified the `tee()` method implementation to avoid this +issue. + +Each branch maintains it's own data buffer. But instead of those buffers containing a +copy of the data, they contain a collection of refcounted references to the data. The +backpressure signaling to the trunk is based on the branch wait the most unconsumed data +in its buffer. ``` +----------------+ @@ -276,6 +285,11 @@ the pending read. ``` +Unfortunately, with this model, we cannot completely avoid the possibility of one branch +reading much slower than the other but we do prevent the memory pileup that would otherwise +occur *so long as the underlying source of the `ReadableStream` is paying proper attention to +the backpressure signaling mechanisms*. + ## Data-flow in an Internal ReadableStream For ***Internal*** streams the implementation is quite different and it is important to diff --git a/src/workerd/api/streams/internal.c++ b/src/workerd/api/streams/internal.c++ index 768bde0646a..7713e805e7e 100644 --- a/src/workerd/api/streams/internal.c++ +++ b/src/workerd/api/streams/internal.c++ @@ -551,18 +551,20 @@ ReadableStreamController::Tee ReadableStreamInternalController::tee(jsg::Lock& j // Create two closed ReadableStreams. return Tee { .branch1 = - jsg::alloc(ReadableStreamInternalController(closed)), + jsg::alloc(kj::heap(closed)), .branch2 = - jsg::alloc(ReadableStreamInternalController(closed)), + jsg::alloc(kj::heap(closed)), }; } KJ_CASE_ONEOF(errored, StreamStates::Errored) { // Create two errored ReadableStreams. return Tee { .branch1 = - jsg::alloc(ReadableStreamInternalController(errored.addRef(js))), + jsg::alloc(kj::heap( + errored.addRef(js))), .branch2 = - jsg::alloc(ReadableStreamInternalController(errored.addRef(js))), + jsg::alloc(kj::heap( + errored.addRef(js))), }; } KJ_CASE_ONEOF(readable, Readable) { diff --git a/src/workerd/api/streams/internal.h b/src/workerd/api/streams/internal.h index 6af3993ac64..87ed8177a71 100644 --- a/src/workerd/api/streams/internal.h +++ b/src/workerd/api/streams/internal.h @@ -122,8 +122,9 @@ class ReadableStreamInternalController: public ReadableStreamController { explicit ReadableStreamInternalController(Readable readable) : state(kj::mv(readable)) {} - ReadableStreamInternalController(ReadableStreamInternalController&& other) = default; - ReadableStreamInternalController& operator=(ReadableStreamInternalController&& other) = default; + KJ_DISALLOW_COPY(ReadableStreamInternalController); + ReadableStreamInternalController(ReadableStreamInternalController&& other) = delete; + ReadableStreamInternalController& operator=(ReadableStreamInternalController&& other) = delete; ~ReadableStreamInternalController() noexcept(false) override; diff --git a/src/workerd/api/streams/queue-test.c++ b/src/workerd/api/streams/queue-test.c++ index 4731352b989..ddd93365329 100644 --- a/src/workerd/api/streams/queue-test.c++ +++ b/src/workerd/api/streams/queue-test.c++ @@ -802,7 +802,7 @@ KJ_TEST("ByteQueue with multiple byob consumers (multi-reads)") { // there should only be two actual BYOB requests // processed by the queue, which will fulfill all four // reads. - MustCall respond([&](jsg::Lock&, auto& pending) { + MustCall respond([&](jsg::Lock&, auto& pending) { static uint counter = 0; auto& req = pending.getRequest(); auto ptr = req.pullInto.store.asArrayPtr().begin(); @@ -812,7 +812,7 @@ KJ_TEST("ByteQueue with multiple byob consumers (multi-reads)") { KJ_ASSERT(pending.isInvalidated()); }, 2); - kj::Maybe> pendingByob; + kj::Maybe> pendingByob; while ((pendingByob = queue.nextPendingByobReadRequest()) != nullptr) { auto& pending = KJ_ASSERT_NONNULL(pendingByob); if (pending->isInvalidated()) { @@ -884,7 +884,7 @@ KJ_TEST("ByteQueue with multiple byob consumers (multi-reads, 2)") { // there should only be two actual BYOB requests // processed by the queue, which will fulfill all four // reads. - MustCall respond([&](jsg::Lock&, auto& pending) { + MustCall respond([&](jsg::Lock&, auto& pending) { static uint counter = 0; auto& req = pending.getRequest(); auto ptr = req.pullInto.store.asArrayPtr().begin(); @@ -894,7 +894,7 @@ KJ_TEST("ByteQueue with multiple byob consumers (multi-reads, 2)") { KJ_ASSERT(pending.isInvalidated()); }, 2); - kj::Maybe> pendingByob; + kj::Maybe> pendingByob; while ((pendingByob = queue.nextPendingByobReadRequest()) != nullptr) { auto& pending = KJ_ASSERT_NONNULL(pendingByob); if (pending->isInvalidated()) { diff --git a/src/workerd/api/streams/queue.c++ b/src/workerd/api/streams/queue.c++ index a189ce79568..be99ec81f09 100644 --- a/src/workerd/api/streams/queue.c++ +++ b/src/workerd/api/streams/queue.c++ @@ -62,11 +62,19 @@ ValueQueue::QueueEntry ValueQueue::QueueEntry::clone() { ValueQueue::Consumer::Consumer( ValueQueue& queue, - ConsumerImpl::StateListener* stateListener) : impl(queue.impl, stateListener) {} + kj::Maybe stateListener) + : impl(queue.impl, stateListener) {} ValueQueue::Consumer::Consumer( QueueImpl& impl, - ConsumerImpl::StateListener* stateListener) : impl(impl, stateListener) {} + kj::Maybe stateListener) + : impl(impl, stateListener) {} + +void ValueQueue::Consumer::cancel( + jsg::Lock& js, + jsg::Optional> maybeReason) { + impl.cancel(js, maybeReason); +} void ValueQueue::Consumer::close(jsg::Lock& js) { impl.close(js); }; @@ -90,12 +98,16 @@ size_t ValueQueue::Consumer::size() { return impl.size(); } kj::Own ValueQueue::Consumer::clone( jsg::Lock& js, - ConsumerImpl::StateListener* stateListener) { + kj::Maybe stateListener) { auto consumer = kj::heap(impl.queue, stateListener); impl.cloneTo(js, consumer->impl); return kj::mv(consumer); } +bool ValueQueue::Consumer::hasReadRequests() { + return impl.hasReadRequests(); +} + #pragma endregion ValueQueue::Consumer ValueQueue::ValueQueue(size_t highWaterMark) : impl(highWaterMark) {} @@ -133,9 +145,8 @@ void ValueQueue::handlePush( // Otherwise, pop the next pending read and resolve it. There should be nothing in the queue. KJ_REQUIRE(state.buffer.empty() && state.queueTotalSize == 0); - auto pending = kj::mv(state.readRequests.front()); + state.readRequests.front().resolve(js, entry->getValue(js)); state.readRequests.pop_front(); - pending.resolve(js, entry->getValue(js)); } void ValueQueue::handleRead( @@ -154,7 +165,6 @@ void ValueQueue::handleRead( KJ_CASE_ONEOF(c, ConsumerImpl::Close) { // The next item was a close sentinel! Resolve the read immediately with a close indicator. request.resolveAsDone(js); - state.readRequests.pop_front(); } KJ_CASE_ONEOF(entry, QueueEntry) { request.resolve(js, entry.entry->getValue(js)); @@ -172,9 +182,14 @@ void ValueQueue::handleRead( // resolved either as soon as there is data available or the consumer closes // or errors. state.readRequests.push_back(kj::mv(request)); + KJ_IF_MAYBE(listener, consumer.stateListener) { + listener->onConsumerWantsData(js); + } } } +size_t ValueQueue::getConsumerCount() { return impl.getConsumerCount(); } + #pragma endregion ValueQueue // ====================================================================================== @@ -183,16 +198,22 @@ void ValueQueue::handleRead( #pragma region ByteQueue::ReadRequest +namespace { +void maybeInvalidateByobRequest(kj::Maybe& req) { + KJ_IF_MAYBE(byobRequest, req) { + byobRequest->invalidate(); + req = nullptr; + } +} +} // namespace + void ByteQueue::ReadRequest::resolveAsDone(jsg::Lock& js) { pullInto.store.trim(pullInto.store.size() - pullInto.filled); resolver.resolve(ReadResult { .value = js.v8Ref(pullInto.store.createHandle(js)), .done = true }); - KJ_IF_MAYBE(byobRequest, byobReadRequest) { - byobRequest->invalidate(); - byobReadRequest = nullptr; - } + maybeInvalidateByobRequest(byobReadRequest); } void ByteQueue::ReadRequest::resolve(jsg::Lock& js) { @@ -201,18 +222,24 @@ void ByteQueue::ReadRequest::resolve(jsg::Lock& js) { .value = js.v8Ref(pullInto.store.createHandle(js)), .done = false }); - KJ_IF_MAYBE(byobRequest, byobReadRequest) { - byobRequest->invalidate(); - byobReadRequest = nullptr; - } + maybeInvalidateByobRequest(byobReadRequest); } void ByteQueue::ReadRequest::reject(jsg::Lock& js, jsg::Value& value) { resolver.reject(value.getHandle(js)); - KJ_IF_MAYBE(byobRequest, byobReadRequest) { - byobRequest->invalidate(); - byobReadRequest = nullptr; - } + maybeInvalidateByobRequest(byobReadRequest); +} + +kj::Own ByteQueue::ReadRequest::makeByobReadRequest( + ConsumerImpl& consumer, + QueueImpl& queue) { + // Why refcounted? One ByobReadRequest reference will be held (eventually) by + // an instance of ReadableStreamBYOBRequest and the other by this ReadRequest. + // Depending on how the read is actually fulfilled, the ByobReadRequest will + // be invalidated by one or the other. + auto req = kj::heap(*this, consumer, queue); + byobReadRequest = *req; + return kj::mv(req); } #pragma endregion ByteQueue::ReadRequest @@ -240,12 +267,22 @@ ByteQueue::QueueEntry ByteQueue::QueueEntry::clone() { #pragma region ByteQueue::Consumer -ByteQueue::Consumer::Consumer(ByteQueue& queue, ConsumerImpl::StateListener* stateListener) +ByteQueue::Consumer::Consumer( + ByteQueue& queue, + kj::Maybe stateListener) : impl(queue.impl, stateListener) {} -ByteQueue::Consumer::Consumer(QueueImpl& impl, ConsumerImpl::StateListener* stateListener) +ByteQueue::Consumer::Consumer( + QueueImpl& impl, + kj::Maybe stateListener) : impl(impl, stateListener) {} +void ByteQueue::Consumer::cancel( + jsg::Lock& js, + jsg::Optional> maybeReason) { + impl.cancel(js, maybeReason); +} + void ByteQueue::Consumer::close(jsg::Lock& js) { impl.close(js); } bool ByteQueue::Consumer::empty() const { return impl.empty(); } @@ -268,17 +305,32 @@ size_t ByteQueue::Consumer::size() const { return impl.size(); } kj::Own ByteQueue::Consumer::clone( jsg::Lock& js, - ConsumerImpl::StateListener* stateListener) { + kj::Maybe stateListener) { auto consumer = kj::heap(impl.queue, stateListener); impl.cloneTo(js, consumer->impl); return kj::mv(consumer); } +bool ByteQueue::Consumer::hasReadRequests() { + return impl.hasReadRequests(); +} + #pragma endregion ByteQueue::Consumer -#pragma region ByteQueue::ByobReadRequest +#pragma region ByteQueue::ByobRequest + +ByteQueue::ByobRequest::~ByobRequest() noexcept(false) { + invalidate(); +} + +void ByteQueue::ByobRequest::invalidate() { + KJ_IF_MAYBE(req, request) { + req->byobReadRequest = nullptr; + request = nullptr; + } +} -void ByteQueue::ByobReadRequest::respond(jsg::Lock& js, size_t amount) { +void ByteQueue::ByobRequest::respond(jsg::Lock& js, size_t amount) { // So what happens here? The read request has been fulfilled directly by writing // into the storage buffer of the request. Unfortunately, this will only resolve // the data for the one consumer from which the request was received. We have to @@ -290,29 +342,88 @@ void ByteQueue::ByobReadRequest::respond(jsg::Lock& js, size_t amount) { // rejected already. auto& req = KJ_REQUIRE_NONNULL(request, "the pending byob read request was already invalidated"); - // It is possible that the request was partially filled already. - req.pullInto.filled += amount; - // The amount cannot be more than the total space in the request store. - KJ_REQUIRE(req.pullInto.filled <= req.pullInto.store.size()); + JSG_REQUIRE(req.pullInto.filled + amount <= req.pullInto.store.size(), RangeError, + kj::str("Too many bytes [", amount ,"] in response to a BYOB read request.")); - // Allocate the entry into which we will be copying the provided data. - auto entry = kj::refcounted(jsg::BackingStore::alloc(js, req.pullInto.filled)); - - // Safely copy the data over into the entry. auto sourcePtr = req.pullInto.store.asArrayPtr(); - std::copy(sourcePtr.begin(), - sourcePtr.begin() + req.pullInto.filled, - entry->toArrayPtr().begin()); - // Push the entry into the other consumers. - queue.push(js, kj::mv(entry), consumer); + if (queue.getConsumerCount() > 1) { + // Allocate the entry into which we will be copying the provided data for the + // other consumers of the queue. + auto entry = kj::refcounted(jsg::BackingStore::alloc(js, amount)); + + // Safely copy the data over into the entry. + std::copy(sourcePtr.begin(), + sourcePtr.begin() + amount, + entry->toArrayPtr().begin()); + + // Push the entry into the other consumers. + queue.push(js, kj::mv(entry), consumer); + } + + // For this consumer, if the number of bytes provided in the response does not + // align with the element size of the read into buffer, we need to shave off + // those extra bytes and push them into the consumers queue so they can be picked + // up by the next read. + req.pullInto.filled += amount; + auto unaligned = req.pullInto.filled % req.pullInto.store.getElementSize(); + // It is possible that the request was partially filled already. + req.pullInto.filled -= unaligned; // Fullfill this request! consumer.resolveRead(js, req); + + if (unaligned > 0) { + auto start = sourcePtr.begin() + (amount - unaligned); + auto excess = kj::refcounted(jsg::BackingStore::alloc(js, unaligned)); + std::copy(start, start + unaligned, excess->toArrayPtr().begin()); + consumer.push(js, kj::mv(excess)); + } } -#pragma endregion ByteQueue::ByobReadRequest +void ByteQueue::ByobRequest::respondWithNewView(jsg::Lock& js, jsg::BufferSource view) { + // The idea here is that rather than filling the view that the controller was given, + // it chose to create it's own view and fill that, likely over the same ArrayBuffer. + // What we do here is perform some basic validations on what we were given, and if + // those pass, we'll replace the backing store held in the req.pullInto with the one + // given, then continue on issuing the respond as normal. + auto& req = KJ_REQUIRE_NONNULL(request, "the pending byob read request was already invalidated"); + auto amount = view.size(); + + JSG_REQUIRE(view.canDetach(js), TypeError, "Unable to use non-detachable ArrayBuffer."); + JSG_REQUIRE(req.pullInto.store.getOffset() + req.pullInto.filled == view.getOffset(), + RangeError, + "The given view has an invalid byte offset."); + JSG_REQUIRE(req.pullInto.store.size() == view.underlyingArrayBufferSize(js), + RangeError, + "The underlying ArrayBuffer is not the correct length."); + JSG_REQUIRE(req.pullInto.filled + amount <= req.pullInto.store.size(), + RangeError, + "The view is not the correct length."); + + req.pullInto.store = view.detach(js); + respond(js, amount); +} + +size_t ByteQueue::ByobRequest::getAtLeast() const { + KJ_IF_MAYBE(req, request) { + return req->pullInto.atLeast; + } + return 0; +} + +v8::Local ByteQueue::ByobRequest::getView(jsg::Lock& js) { + KJ_IF_MAYBE(req, request) { + return req->pullInto.store.getTypedViewSlice( + req->pullInto.filled, + req->pullInto.store.size() + ).createHandle(js).As(); + } + return v8::Local(); +} + +#pragma endregion ByteQueue::ByobRequest ByteQueue::ByteQueue(size_t highWaterMark) : impl(highWaterMark) {} @@ -475,9 +586,9 @@ void ByteQueue::handlePush( amountAvailable -= amountToCopy; entryOffset += amountToCopy; pending.pullInto.filled += amountToCopy; - auto released = kj::mv(pending); + + pending.resolve(js); state.readRequests.pop_front(); - released.resolve(js); } // If the entry was consumed completely by the pending read, then we're done! @@ -504,26 +615,18 @@ void ByteQueue::handleRead( bool isByob = request.pullInto.type == ReadRequest::Type::BYOB; state.readRequests.push_back(kj::mv(request)); if (isByob) { + // Because ReadRequest is movable, and because the ByobRequest captures + // a reference to the ReadRequest, we wait until after it is added to + // state.readRequests to create the associated ByobRequest. KJ_REQUIRE_NONNULL(queue.getState()).pendingByobReadRequests.push_back( - kj::heap(state.readRequests.back(), consumer, queue)); + state.readRequests.back().makeByobReadRequest(consumer, queue)); } - }; - - // If there are no pending read requests and there is data in the buffer, - // we will try to fulfill the read request immediately. - if (state.readRequests.empty() && state.queueTotalSize > 0) { - // If the available size is less than the read requests atLeast, then - // push the read request into the pending so we can wait for more data. - if (state.queueTotalSize < request.pullInto.atLeast) { - return pendingRead(); + KJ_IF_MAYBE(listener, consumer.stateListener) { + listener->onConsumerWantsData(js); } + }; - // Awesome, ok, it looks like we have enough data in the queue for us - // to minimally fill this read request! The amount to copy is the lesser - // of the queue total size and the maximum amount of space in the request - // pull into. - auto amountToConsume = kj::min(state.queueTotalSize, request.pullInto.store.size()); - + const auto consume = [&](size_t amountToConsume) { while (amountToConsume > 0) { KJ_REQUIRE(!state.buffer.empty()); // There must be at least one item in the buffer. @@ -532,9 +635,7 @@ void ByteQueue::handleRead( KJ_SWITCH_ONEOF(item) { KJ_CASE_ONEOF(c, ConsumerImpl::Close) { // We reached the end of the buffer! All data has been consumed. - // We want to resolve the read request with everything we have - // so far and transition the consumer into the closed state. - return request.resolveAsDone(js); + return true; } KJ_CASE_ONEOF(entry, QueueEntry) { // The amount to copy is the lesser of the current entry size minus @@ -572,6 +673,34 @@ void ByteQueue::handleRead( } } } + return false; + }; + + // If there are no pending read requests and there is data in the buffer, + // we will try to fulfill the read request immediately. + if (state.readRequests.empty() && state.queueTotalSize > 0) { + // If the available size is less than the read requests atLeast, then + // push the read request into the pending so we can wait for more data... + if (state.queueTotalSize < request.pullInto.atLeast) { + // If there is anything in the consumers queue at this point, We need to + // copy those bytes into the byob buffer and advance the filled counter + // forward that number of bytes. + if (state.queueTotalSize > 0 && consume(state.queueTotalSize)) { + return request.resolveAsDone(js); + } + + return pendingRead(); + } + + // Awesome, ok, it looks like we have enough data in the queue for us + // to minimally fill this read request! The amount to copy is the lesser + // of the queue total size and the maximum amount of space in the request + // pull into. + if (consume(kj::min(state.queueTotalSize, request.pullInto.store.size()))) { + // If consume returns true, the consumer hit the end and we need to + // just resolve the request as done and return. + return request.resolveAsDone(js); + } // Now, we can resolve the read promise. Since we consumed data from the // buffer, we also want to make sure to notify the queue so it can update @@ -591,7 +720,7 @@ void ByteQueue::handleRead( } } -kj::Maybe> ByteQueue::nextPendingByobReadRequest() { +kj::Maybe> ByteQueue::nextPendingByobReadRequest() { KJ_IF_MAYBE(state, impl.getState()) { while (!state->pendingByobReadRequests.empty()) { auto request = kj::mv(state->pendingByobReadRequests.front()); @@ -604,6 +733,20 @@ kj::Maybe> ByteQueue::nextPendingByobReadReq return nullptr; } +bool ByteQueue::hasPartiallyFulfilledRead() { + KJ_IF_MAYBE(state, impl.getState()) { + if (!state->pendingByobReadRequests.empty()) { + auto& pending = state->pendingByobReadRequests.front(); + if (!pending->isInvalidated() && pending->getRequest().pullInto.filled > 0) { + return true; + } + } + } + return false; +} + +size_t ByteQueue::getConsumerCount() { return impl.getConsumerCount(); } + #pragma endregion ByteQueue } // namespace workerd::api diff --git a/src/workerd/api/streams/queue.h b/src/workerd/api/streams/queue.h index 14cdeeeba2e..2aafa7c903f 100644 --- a/src/workerd/api/streams/queue.h +++ b/src/workerd/api/streams/queue.h @@ -163,7 +163,7 @@ class QueueImpl final { // If we are already closed or errored, do nothing here. KJ_IF_MAYBE(ready, state.template tryGet()) { for (auto& consumer : ready->consumers) { - consumer.get().close(js); + consumer.ref->close(js); } state.template init(); } @@ -184,7 +184,7 @@ class QueueImpl final { // If we are already closed or errored, do nothing here. KJ_IF_MAYBE(ready, state.template tryGet()) { for (auto& consumer : ready->consumers) { - consumer.get().error(js, reason.addRef(js)); + consumer.ref->error(js, reason.addRef(js)); } state = kj::mv(reason); } @@ -197,7 +197,7 @@ class QueueImpl final { totalQueueSize = 0; KJ_IF_MAYBE(ready, state.template tryGet()) { for (auto& consumer : ready->consumers) { - totalQueueSize = kj::max(totalQueueSize, consumer.get().size()); + totalQueueSize = kj::max(totalQueueSize, consumer.ref->size()); } } } @@ -215,18 +215,41 @@ class QueueImpl final { for (auto& consumer : ready.consumers) { KJ_IF_MAYBE(skip, skipConsumer) { - if (&consumer.get() == &(*skip)) { + if (consumer.ref == &(*skip)) { continue; } } - consumer.get().push(js, kj::addRef(*entry)); + consumer.ref->push(js, kj::addRef(*entry)); } } size_t size() const { return totalQueueSize; } // The current size of consumer with the most stored data. + size_t getConsumerCount() const { + KJ_SWITCH_ONEOF(state) { + KJ_CASE_ONEOF(closed, Closed) { return 0; } + KJ_CASE_ONEOF(errored, Errored) { return 0; } + KJ_CASE_ONEOF(ready, Ready) { return ready.consumers.size(); } + } + KJ_UNREACHABLE; + } + + bool wantsRead() const { + KJ_SWITCH_ONEOF(state) { + KJ_CASE_ONEOF(closed, Closed) { return false; } + KJ_CASE_ONEOF(errored, Errored) { return false; } + KJ_CASE_ONEOF(ready, Ready) { + for (auto& consumer : ready.consumers) { + if (consumer.ref->hasReadRequests()) return true; + } + return false; + } + } + KJ_UNREACHABLE; + } + kj::Maybe getState() KJ_LIFETIMEBOUND { // Specific queue implementations may provide additional state that is attached // to the Ready struct. @@ -241,14 +264,12 @@ class QueueImpl final { using Errored = jsg::Value; struct ConsumerRef { - kj::Maybe ref; - // The kj::Maybe here is used only to make ConsumerRef trivially movable. + ConsumerImpl* ref; bool operator==(ConsumerRef& other) const { return hashCode() == other.hashCode(); } - ConsumerImpl& get() const { return KJ_ASSERT_NONNULL(ref); } auto hashCode() const { - return kj::hashCode(&get()); + return kj::hashCode(ref); } }; @@ -260,13 +281,13 @@ class QueueImpl final { size_t totalQueueSize = 0; kj::OneOf state = Ready(); - void addConsumer(ConsumerImpl& consumer) { - auto& ready = KJ_REQUIRE_NONNULL(state.template tryGet(), - "The queue is closed or errored."); - ready.consumers.insert(ConsumerRef { .ref = consumer }); + void addConsumer(ConsumerImpl* consumer) { + KJ_IF_MAYBE(ready, state.template tryGet()) { + ready->consumers.insert(ConsumerRef { .ref = consumer }); + } } - void removeConsumer(ConsumerImpl& consumer) { + void removeConsumer(ConsumerImpl* consumer) { KJ_IF_MAYBE(ready, state.template tryGet()) { ready->consumers.eraseMatch(ConsumerRef { .ref = consumer }); maybeUpdateBackpressure(); @@ -282,8 +303,9 @@ class ConsumerImpl final { // Provides the underlying implementation shared by ByteQueue::Consumer and ValueQueue::Consumer public: struct StateListener { - virtual void onConsumerClose() = 0; - virtual void onConsumerError(jsg::Value reason) = 0; + virtual void onConsumerClose(jsg::Lock& js) = 0; + virtual void onConsumerError(jsg::Lock& js, jsg::Value reason) = 0; + virtual void onConsumerWantsData(jsg::Lock& js) = 0; }; using QueueImpl = QueueImpl; @@ -304,9 +326,9 @@ class ConsumerImpl final { using Entry = typename Self::Entry; using QueueEntry = typename Self::QueueEntry; - ConsumerImpl(QueueImpl& queue, StateListener* stateListener = nullptr) + ConsumerImpl(QueueImpl& queue, kj::Maybe stateListener = nullptr) : queue(queue), stateListener(stateListener) { - queue.addConsumer(*this); + queue.addConsumer(this); } ConsumerImpl(ConsumerImpl& other) = delete; @@ -315,7 +337,20 @@ class ConsumerImpl final { ConsumerImpl& operator=(ConsumerImpl&&) = delete; ~ConsumerImpl() noexcept(false) { - queue.removeConsumer(*this); + queue.removeConsumer(this); + } + + void cancel(jsg::Lock& js, jsg::Optional> maybeReason) { + KJ_SWITCH_ONEOF(state) { + KJ_CASE_ONEOF(closed, Closed) {} + KJ_CASE_ONEOF(errored, Errored) {} + KJ_CASE_ONEOF(ready, Ready) { + for (auto& request : ready.readRequests) { + request.resolveAsDone(js); + } + state.template init(); + } + } } void close(jsg::Lock& js) { @@ -438,7 +473,27 @@ class ConsumerImpl final { KJ_CASE_ONEOF(closed, Closed) { return false; } KJ_CASE_ONEOF(errored, Errored) { return false; } KJ_CASE_ONEOF(ready, Ready) { - return ready.readRequests.empty(); + return !ready.readRequests.empty(); + } + } + KJ_UNREACHABLE; + } + + void visitForGc(jsg::GcVisitor& visitor) { + KJ_SWITCH_ONEOF(state) { + KJ_CASE_ONEOF(closed, Closed) {} + KJ_CASE_ONEOF(errored, Errored) { + visitor.visit(errored); + } + KJ_CASE_ONEOF(ready, Ready) { + for (auto& entry : ready.buffer) { + KJ_IF_MAYBE(e, entry.template tryGet()) { + visitor.visit(*e); + } + } + for (auto& req : ready.readRequests) { + visitor.visit(req.resolver); + } } } } @@ -457,7 +512,7 @@ class ConsumerImpl final { QueueImpl& queue; kj::OneOf state = Ready(); - StateListener* stateListener; + kj::Maybe stateListener; bool isClosing() { // Closing state is determined by whether there is a Close sentinel that has been @@ -486,10 +541,12 @@ class ConsumerImpl final { for (auto& request : ready->readRequests) { request.reject(js, *reason); } - state = kj::mv(*reason); - if (stateListener != nullptr) { - stateListener->onConsumerError(reason->addRef(js)); - stateListener = nullptr; + state = reason->addRef(js); + KJ_IF_MAYBE(listener, stateListener) { + listener->onConsumerError(js, kj::mv(*reason)); + // After this point, we should not assume that this consumer can + // be safely used at all. It's most likely the stateListener has + // released it. } } else { // Otherwise, if the buffer is empty isClosing() is true, resolve the @@ -501,9 +558,11 @@ class ConsumerImpl final { request.resolveAsDone(js); } state.template init(); - if (stateListener != nullptr) { - stateListener->onConsumerClose(); - stateListener = nullptr; + KJ_IF_MAYBE(listener, stateListener) { + listener->onConsumerClose(js); + // After this point, we should not assume that this consumer can + // be safely used at all. It's most likely the stateListener has + // released it. } } } @@ -554,17 +613,23 @@ class ValueQueue final { struct QueueEntry { kj::Own entry; QueueEntry clone(); + + void visitForGc(jsg::GcVisitor& visitor) { + if (entry) visitor.visit(*entry); + } }; class Consumer final { public: - Consumer(ValueQueue& queue, ConsumerImpl::StateListener* stateListener = nullptr); - Consumer(QueueImpl& queue, ConsumerImpl::StateListener* stateListener = nullptr); + Consumer(ValueQueue& queue, kj::Maybe stateListener = nullptr); + Consumer(QueueImpl& queue, kj::Maybe stateListener = nullptr); Consumer(Consumer&&) = delete; Consumer(Consumer&) = delete; Consumer& operator=(Consumer&&) = delete; Consumer& operator=(Consumer&) = delete; + void cancel(jsg::Lock& js, jsg::Optional> maybeReason); + void close(jsg::Lock& js); bool empty(); @@ -579,7 +644,14 @@ class ValueQueue final { size_t size(); - kj::Own clone(jsg::Lock& js, ConsumerImpl::StateListener* stateListener = nullptr); + kj::Own clone(jsg::Lock& js, + kj::Maybe stateListener = nullptr); + + bool hasReadRequests(); + + void visitForGc(jsg::GcVisitor& visitor) { + visitor.visit(impl); + } private: ConsumerImpl impl; @@ -601,6 +673,19 @@ class ValueQueue final { size_t size() const; + size_t getConsumerCount(); + + bool wantsRead() const { + return impl.wantsRead(); + } + + bool hasPartiallyFulfilledRead() { + // A ValueQueue can never have a partially fulfilled read. + return false; + } + + void visitForGc(jsg::GcVisitor& visitor) {} + private: QueueImpl impl; @@ -626,12 +711,16 @@ class ByteQueue final { using ConsumerImpl = ConsumerImpl; using QueueImpl = QueueImpl; - class ByobReadRequest; + class ByobRequest; struct ReadRequest final { enum class Type { DEFAULT, BYOB }; jsg::Promise::Resolver resolver; - kj::Maybe byobReadRequest; + kj::Maybe byobReadRequest; + // The reference here should be cleared when the ByobRequest is invalidated, + // which happens either when respond(), respondWithNewView(), or invalidate() + // is called, or when the ByobRequest is destroyed, whichever comes first. + struct { jsg::BackingStore store; size_t filled = 0; @@ -642,28 +731,48 @@ class ByteQueue final { void resolveAsDone(jsg::Lock& js); void resolve(jsg::Lock& js); void reject(jsg::Lock& js, jsg::Value& value); + + kj::Own makeByobReadRequest(ConsumerImpl& consumer, QueueImpl& queue); }; - class ByobReadRequest final { + class ByobRequest final { + // The ByobRequest is essentially a handle to the ByteQueue::ReadRequest that can be given to a + // ReadableStreamBYOBRequest object to fulfill the request using the BYOB API pattern. + // + // When isInvalidated() is false, respond() or respondWithNewView() can be called to fulfill + // the BYOB read request. Once either of those are called, or once invalidate() is called, + // the ByobRequest is no longer usable and should be discarded. public: - ByobReadRequest( + ByobRequest( ReadRequest& request, ConsumerImpl& consumer, QueueImpl& queue) : request(request), consumer(consumer), - queue(queue) { - request.byobReadRequest = *this; - } + queue(queue) {} + + KJ_DISALLOW_COPY(ByobRequest); + ByobRequest(ByobRequest&&) = delete; + ByobRequest& operator=(ByobRequest&&) = delete; + + ~ByobRequest() noexcept(false); ReadRequest& getRequest() { return KJ_ASSERT_NONNULL(request); } void respond(jsg::Lock& js, size_t amount); - inline void invalidate() { request = nullptr; } + void respondWithNewView(jsg::Lock& js, jsg::BufferSource view); + + void invalidate(); + // Disconnects this ByobRequest instance from the associated ByteQueue::ReadRequest. + // The term "invalidate" is adopted from the streams spec for handling BYOB requests. inline bool isInvalidated() const { return request == nullptr; } + size_t getAtLeast() const; + + v8::Local getView(jsg::Lock& js); + private: kj::Maybe request; ConsumerImpl& consumer; @@ -671,7 +780,7 @@ class ByteQueue final { }; struct State { - std::deque> pendingByobReadRequests; + std::deque> pendingByobReadRequests; }; class Entry final: public kj::Refcounted { @@ -695,17 +804,21 @@ class ByteQueue final { size_t offset; QueueEntry clone(); + + void visitForGc(jsg::GcVisitor& visitor) {} }; class Consumer { public: - Consumer(ByteQueue& queue, ConsumerImpl::StateListener* stateListener = nullptr); - Consumer(QueueImpl& queue, ConsumerImpl::StateListener* stateListener = nullptr); + Consumer(ByteQueue& queue, kj::Maybe stateListener = nullptr); + Consumer(QueueImpl& queue, kj::Maybe stateListener = nullptr); Consumer(Consumer&&) = delete; Consumer(Consumer&) = delete; Consumer& operator=(Consumer&&) = delete; Consumer& operator=(Consumer&) = delete; + void cancel(jsg::Lock& js, jsg::Optional> maybeReason); + void close(jsg::Lock& js); bool empty() const; @@ -720,7 +833,14 @@ class ByteQueue final { size_t size() const; - kj::Own clone(jsg::Lock& js, ConsumerImpl::StateListener* stateListener = nullptr); + kj::Own clone(jsg::Lock& js, + kj::Maybe stateListener = nullptr); + + bool hasReadRequests(); + + void visitForGc(jsg::GcVisitor& visitor) { + visitor.visit(impl); + } private: ConsumerImpl impl; @@ -740,7 +860,15 @@ class ByteQueue final { size_t size() const; - kj::Maybe> nextPendingByobReadRequest(); + size_t getConsumerCount(); + + bool wantsRead() const { + return impl.wantsRead(); + } + + bool hasPartiallyFulfilledRead(); + + kj::Maybe> nextPendingByobReadRequest(); // nextPendingByobReadRequest will be used to support the ReadableStreamBYOBRequest interface // that is part of ReadableByteStreamController. When user code calls the `controller.byobRequest` // API on a ReadableByteStreamController, they are going to get an instance of a @@ -750,6 +878,8 @@ class ByteQueue final { // their lifespan to be attached to the ReadableStreamBYOBRequest object but internally they // will be disconnected as appropriate. + void visitForGc(jsg::GcVisitor& visitor) {} + private: QueueImpl impl; diff --git a/src/workerd/api/streams/readable.c++ b/src/workerd/api/streams/readable.c++ index 1c277686b66..ef8de31268d 100644 --- a/src/workerd/api/streams/readable.c++ +++ b/src/workerd/api/streams/readable.c++ @@ -291,7 +291,7 @@ void ReadableStreamBYOBReader::visitForGc(jsg::GcVisitor& visitor) { ReadableStream::ReadableStream( IoContext& ioContext, kj::Own source) - : controller(ReadableStreamInternalController(ioContext.addObject(kj::mv(source)))) { + : controller(kj::heap(ioContext.addObject(kj::mv(source)))) { getController().setOwnerRef(*this); } @@ -301,14 +301,11 @@ ReadableStream::ReadableStream(Controller controller) : controller(kj::mv(contro ReadableStreamController& ReadableStream::getController() { KJ_SWITCH_ONEOF(controller) { - KJ_CASE_ONEOF(c, ReadableStreamInternalController) { - return c; + KJ_CASE_ONEOF(c, kj::Own) { + return *c; } - KJ_CASE_ONEOF(c, ReadableStreamJsController) { - return c; - } - KJ_CASE_ONEOF(c, ReadableStreamJsTeeController) { - return c; + KJ_CASE_ONEOF(c, kj::Own) { + return *c; } } KJ_UNREACHABLE; @@ -460,7 +457,7 @@ jsg::Ref ReadableStream::constructor( "To use the new ReadableStream() constructor, enable the " "streams_enable_constructors feature flag."); - auto stream = jsg::alloc(ReadableStreamJsController()); + auto stream = jsg::alloc(kj::heap()); static_cast( stream->getController()).setup(js, kj::mv(underlyingSource), kj::mv(queuingStrategy)); return kj::mv(stream); diff --git a/src/workerd/api/streams/readable.h b/src/workerd/api/streams/readable.h index 51ea9ec7dd1..27b7fd78267 100644 --- a/src/workerd/api/streams/readable.h +++ b/src/workerd/api/streams/readable.h @@ -180,9 +180,8 @@ class ReadableStream: public jsg::Object { jsg::Optional value); public: - using Controller = kj::OneOf; + using Controller = kj::OneOf, + kj::Own>; explicit ReadableStream(IoContext& ioContext, kj::Own source); diff --git a/src/workerd/api/streams/standard.c++ b/src/workerd/api/streams/standard.c++ index d9d382e6e17..645fc0c10e1 100644 --- a/src/workerd/api/streams/standard.c++ +++ b/src/workerd/api/streams/standard.c++ @@ -37,22 +37,6 @@ jsg::Promise maybeRunAlgorithm( return js.resolvedPromise(); } -kj::Maybe getChunkSize( - jsg::Lock& js, - auto& sizeAlgorithm, - v8::Local value, - auto onError) { - KJ_IF_MAYBE(alg, sizeAlgorithm) { - return js.tryCatch([&]() -> kj::Maybe { - return (*alg)(js, value); - }, [&](jsg::Value&& exception) -> kj::Maybe { - onError(js, exception.getHandle(js)); - return nullptr; - }); - } - return 1; -} - // ====================================================================================== template @@ -133,12 +117,37 @@ void ReadableLockImpl::visitForGc(jsg::GcVisitor& visitor) { KJ_CASE_ONEOF(locked, PipeLocked) { visitor.visit(locked); } - KJ_CASE_ONEOF(locked, TeeLocked) { + KJ_CASE_ONEOF(locked, ReaderLocked) { visitor.visit(locked); } + } +} + +template +void ReadableLockImpl::onClose() { + KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(locked, ReaderLocked) { - visitor.visit(locked); + maybeResolvePromise(locked.getClosedFulfiller()); + } + KJ_CASE_ONEOF(locked, ReadableLockImpl::PipeLocked) { + state.template init(); + } + KJ_CASE_ONEOF(locked, Locked) {} + KJ_CASE_ONEOF(locked, Unlocked) {} + } +} + +template +void ReadableLockImpl::onError(jsg::Lock& js, v8::Local reason) { + KJ_SWITCH_ONEOF(state) { + KJ_CASE_ONEOF(locked, ReaderLocked) { + maybeRejectPromise(locked.getClosedFulfiller(), reason); + } + KJ_CASE_ONEOF(locked, ReadableLockImpl::PipeLocked) { + state.template init(); } + KJ_CASE_ONEOF(locked, Locked) {} + KJ_CASE_ONEOF(locked, Unlocked) {} } } @@ -159,111 +168,6 @@ void ReadableLockImpl::PipeLocked::visitForGc(jsg::GcVisitor &visito visitor.visit(writableStreamRef); } -template -kj::Maybe ReadableLockImpl::tryTeeLock( - Controller& self) { -// TODO(stream-tee): There will be no more tee controller and we have to rethink tee lock. -// Essentially, tee lock will mean the stream is effectively closed and no longer used because -// the underlying controller references have been passed on to the branches. - if (isLockedToReader()) { - return nullptr; - } - state.template init(self); - return state.template get(); -} - -// TODO(stream-tee): Everything relating to TeeLocked here goes away. - -template -void ReadableLockImpl::TeeLocked::addBranch(Branch* branch) { - KJ_ASSERT(branches.find(BranchPtr(branch)) == nullptr, - "branch should not already be in the list!"); - branches.insert(BranchPtr(branch)); -} - -template -void ReadableLockImpl::TeeLocked::close() { - inner.state.template init(); - forEachBranch([](auto& branch) { branch.doClose(); }); -} - -template -void ReadableLockImpl::TeeLocked::error( - jsg::Lock& js, - v8::Local reason) { - inner.state.template init(js.v8Ref(reason)); - forEachBranch([&](auto& branch) { branch.doError(js, reason); }); - // Each of the branches should have removed themselves from the tee adapter - // Let's make sure. - KJ_ASSERT(branches.size() == 0); -} - -template -void ReadableLockImpl::TeeLocked::ensurePulling(jsg::Lock& js) { - KJ_IF_MAYBE(pulling, maybePulling) { - pullAgain = true; - return; - } - - maybePulling = pull(js).then(js, - JSG_VISITABLE_LAMBDA((this, ref = inner.addRef()), (ref), - (jsg::Lock& js, ReadResult result) { - maybePulling = nullptr; - - forEachBranch([&](auto& branch) { - branch.handleData(js, ReadResult { - .value = result.value.map([&](jsg::Value& ref) -> jsg::Value { - return ref.addRef(js); - }), - .done = result.done, - }); - }); - - if (pullAgain) { - pullAgain = false; - ensurePulling(js); - } - return js.resolvedPromise(); - }), JSG_VISITABLE_LAMBDA((this, ref = inner.addRef()), - (ref), - (jsg::Lock& js, jsg::Value value) { - maybePulling = nullptr; - return js.rejectedPromise(kj::mv(value)); - })); -} - -template -jsg::Promise ReadableLockImpl::TeeLocked::pull(jsg::Lock& js) { - if (inner.state.template is()) { - return js.resolvedPromise(ReadResult { .done = true }); - } - - KJ_IF_MAYBE(errored, inner.state.template tryGet()) { - return js.rejectedPromise(errored->addRef(js)); - } - - return KJ_ASSERT_NONNULL(inner.read(js, nullptr)); -} - -template -void ReadableLockImpl::TeeLocked::removeBranch( - Branch* branch, - kj::Maybe maybeJs) { - KJ_ASSERT(branches.eraseMatch(BranchPtr(branch)), - "Tee branch wasn't found? Possible invalid branch pointer."); - - KJ_IF_MAYBE(js, maybeJs) { - if (branches.size() == 0) { - inner.doCancel(*js, js->v8Undefined()); - } - } -} - -template -void ReadableLockImpl::TeeLocked::visitForGc(jsg::GcVisitor &visitor) { - visitor.visit(maybePulling); -} - // ====================================================================================== template @@ -402,45 +306,98 @@ kj::Maybe> WritableLockImpl::PipeLocked::checkSig // ====================================================================================== +namespace { +int getHighWaterMark(const UnderlyingSource& underlyingSource, + const StreamQueuingStrategy& queuingStrategy) { + bool isBytes = underlyingSource.type.map([](auto& s) { return s == "bytes"; }).orDefault(false); + return queuingStrategy.highWaterMark.orDefault(isBytes ? 0 : 1); +} +} // namespace + +template +ReadableImpl::ReadableImpl( + UnderlyingSource underlyingSource, + StreamQueuingStrategy queuingStrategy) + : state(Queue(getHighWaterMark(underlyingSource, queuingStrategy))), + algorithms(kj::mv(underlyingSource), kj::mv(queuingStrategy)) {} + +template +void ReadableImpl::start(jsg::Lock& js, jsg::Ref self) { + KJ_ASSERT(!started && algorithms.starting == nullptr); + + auto onSuccess = JSG_VISITABLE_LAMBDA((this, self = self.addRef()), (self), (jsg::Lock& js) { + algorithms.starting = nullptr; + started = true; + pullIfNeeded(js, kj::mv(self)); + }); + + auto onFailure = JSG_VISITABLE_LAMBDA((this, self = self.addRef()), (self), + (jsg::Lock& js, jsg::Value reason) { + algorithms.starting = nullptr; + started = true; + doError(js, kj::mv(reason)); + }); + + algorithms.starting = maybeRunAlgorithm(js, + algorithms.start, + kj::mv(onSuccess), + kj::mv(onFailure), + kj::mv(self)); + algorithms.start = nullptr; +} + template jsg::Promise ReadableImpl::cancel( jsg::Lock& js, jsg::Ref self, v8::Local reason) { - KJ_ASSERT(state.template is()); - KJ_IF_MAYBE(pendingCancel, maybePendingCancel) { + KJ_SWITCH_ONEOF(state) { + KJ_CASE_ONEOF(closed, StreamStates::Closed) { + // We are already closed. There's nothing to cancel. + // This shouldn't happen but we handle the case anyway, just to be safe. + return js.resolvedPromise(); + } + KJ_CASE_ONEOF(errored, StreamStates::Errored) { + // We are already errored. There's nothing to cancel. + // This shouldn't happen but we handle the case anyway, just to be safe. + return js.rejectedPromise(errored.getHandle(js)); + } + KJ_CASE_ONEOF(queue, Queue) { + size_t consumerCount = queue.getConsumerCount(); + if (consumerCount > 1) { + // If there is more than 1 consumer, then we just return here with an + // immediately resolved promise. The consumer will remove itself, + // canceling it's interest in the underlying source but we do not yet + // want to cancel the underlying source since there are still other + // consumers that want data. + return js.resolvedPromise(); + } -// TODO(stream-tee): This should only be called if the last consumer known to the queue -// is canceling. + // Otherwise, there should be exactly one consumer at this point. + KJ_ASSERT(consumerCount == 1); + KJ_IF_MAYBE(pendingCancel, maybePendingCancel) { + // If we're already waiting for cancel to complete, just return the + // already existing pending promise. + // This shouldn't happen but we handle the case anyway, just to be safe. + return pendingCancel->promise.whenResolved(); + } - // If we're already waiting for cancel to complete, just return the - // already existing pending promise. - return pendingCancel->promise.whenResolved(); + auto prp = js.newPromiseAndResolver(); + maybePendingCancel = PendingCancel { + .fulfiller = kj::mv(prp.resolver), + .promise = kj::mv(prp.promise), + }; + auto promise = KJ_ASSERT_NONNULL(maybePendingCancel).promise.whenResolved(); + doCancel(js, kj::mv(self), reason); + return kj::mv(promise); + } } - - auto prp = js.newPromiseAndResolver(); - maybePendingCancel = PendingCancel { - .fulfiller = kj::mv(prp.resolver), - .promise = kj::mv(prp.promise), - }; - auto promise = KJ_ASSERT_NONNULL(maybePendingCancel).promise.whenResolved(); - doCancel(js, kj::mv(self), reason); - return kj::mv(promise); + KJ_UNREACHABLE; } template bool ReadableImpl::canCloseOrEnqueue() { - return owner != nullptr && state.template is() && !closeRequested; -} - -template -ReadRequest ReadableImpl::dequeueReadRequest() { -// TODO(stream-tee): This goes away. The Queue::Consumer is responsible for tracking -// read requests. - KJ_ASSERT(!readRequests.empty()); - auto request = kj::mv(readRequests.front()); - readRequests.pop_front(); - return kj::mv(request); + return state.template is() && !closeRequested; } template @@ -448,10 +405,13 @@ void ReadableImpl::doCancel( jsg::Lock& js, jsg::Ref self, v8::Local reason) { - if (!state.template is()) { - return; - } - queue.reset(); + // doCancel() is triggered by cancel() being called, which is an explicit signal from + // the ReadableStream that we don't care about the data this controller provides any + // more. We don't need to notify the consumers because we presume they already know + // that they called cancel. What we do want to do here, tho, is close the implementation + // and trigger the cancel algorithm. + + state.template init(); auto onSuccess = JSG_VISITABLE_LAMBDA((this, self = self.addRef()), (self), @@ -466,6 +426,9 @@ void ReadableImpl::doCancel( (self), (jsg::Lock& js, jsg::Value reason) { algorithms.canceling = nullptr; + // We do not call doError() here because there's really no point. Everything + // that cares about the state of this controller impl has signaled that it + // no longer cares and has gone away. doClose(js); KJ_IF_MAYBE(pendingCancel, maybePendingCancel) { maybeRejectPromise(pendingCancel->fulfiller, reason.getHandle(js)); @@ -480,59 +443,62 @@ void ReadableImpl::doCancel( } template -void ReadableImpl::doClose(jsg::Lock& js) { -// TODO(stream-tee): Closing and resetting the queue also needs to notify all of the -// still-attached consumers that they should close and detach immediately. All still -// pending reads should be closed out (if they are partially fulfilled) or canceled, -// and the references holding this controller should be cleared. - if (!state.template is()) { +void ReadableImpl::enqueue(jsg::Lock& js, kj::Own entry, jsg::Ref self) { + JSG_REQUIRE(canCloseOrEnqueue(), TypeError, "This ReadableStream is closed."); + KJ_DEFER(pullIfNeeded(js, kj::mv(self))); + auto& queue = state.template get(); + queue.push(js, kj::mv(entry)); +} + +template +void ReadableImpl::close(jsg::Lock& js) { + JSG_REQUIRE(canCloseOrEnqueue(), TypeError, "This ReadableStream is closed."); + auto& queue = state.template get(); + + if (queue.hasPartiallyFulfilledRead()) { + auto error = js.v8Ref(js.v8TypeError( + "This ReadableStream was closed with a partial read pending.")); + doError(js, error.addRef(js)); + js.throwException(kj::mv(error)); return; } - state.template init(); - queue.reset(); - algorithms.clear(); - for (auto& request : readRequests) { - request.resolve(ReadResult { .done = true }); - } + queue.close(js); - KJ_IF_MAYBE(theOwner, owner) { - theOwner->doClose(); - owner = nullptr; - // Calling doClose here most likely caused the ReadableImpl to be destroyed, - // so it is important not to do anything else after calling doClose here. - } + state.template init(); + doClose(js); } template -void ReadableImpl::doError(jsg::Lock& js, v8::Local reason) { -// TODO(stream-tee): Error the queue, notify the consumers of the error. All still -// pending reads should be rejected and all still connected consumers should detach, -// keeping record of the error. All references holding this controller should be cleared. - if (!state.template is()) { - return; - } - state = js.v8Ref(reason); - queue.reset(); +void ReadableImpl::doClose(jsg::Lock& js) { + // The state should have already been set to closed. + KJ_ASSERT(state.template is()); algorithms.clear(); +} - while (!readRequests.empty()) { - auto request = kj::mv(readRequests.front()); - readRequests.pop_front(); - request.reject(reason); - } - - KJ_IF_MAYBE(theOwner, owner) { - theOwner->doError(js, reason); - owner = nullptr; - // Calling doError here most likely caused the ReadableImpl to be destroyed, - // so it is important not to do anything else after calling doError here. +template +void ReadableImpl::doError(jsg::Lock& js, jsg::Value reason) { + KJ_SWITCH_ONEOF(state) { + KJ_CASE_ONEOF(closed, StreamStates::Closed) { + // We're already closed, so we really don't care if there was an error. Do nothing. + return; + } + KJ_CASE_ONEOF(errored, StreamStates::Errored) { + // We're already errored, so we really don't care if there was an error. Do nothing. + return; + } + KJ_CASE_ONEOF(queue, Queue) { + queue.error(js, reason.addRef(js)); + state = kj::mv(reason); + algorithms.clear(); + return; + } } + KJ_UNREACHABLE; } template kj::Maybe ReadableImpl::getDesiredSize() { -// TODO(stream-tee): Reimplemented by the queue KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return 0; @@ -540,8 +506,8 @@ kj::Maybe ReadableImpl::getDesiredSize() { KJ_CASE_ONEOF(errored, StreamStates::Errored) { return nullptr; } - KJ_CASE_ONEOF(readable, Readable) { - return highWaterMark - queue.size(); + KJ_CASE_ONEOF(queue, Queue) { + return queue.desiredSize(); } } KJ_UNREACHABLE; @@ -549,18 +515,10 @@ kj::Maybe ReadableImpl::getDesiredSize() { template bool ReadableImpl::shouldCallPull() { - if (!canCloseOrEnqueue()) { - return false; - } - if (!started) { - return false; - } -// TODO(stream-tee): This will need to be reimplemented to see if any of the known -// consumers want data. - if (getOwner().isLocked() && readRequests.size() > 0) { - return true; - } - return getDesiredSize().orDefault(1) > 0; + // We should call pull if any of the consumers known to the queue have read requests or + // we haven't yet signalled backpressure. + return canCloseOrEnqueue() && + (state.template get().wantsRead() || getDesiredSize().orDefault(0) > 0); } template @@ -590,7 +548,7 @@ void ReadableImpl::pullIfNeeded(jsg::Lock& js, jsg::Ref self) { auto onFailure = JSG_VISITABLE_LAMBDA((this, self = self.addRef()), (self), (jsg::Lock& js, jsg::Value reason) { algorithms.pulling = nullptr; - doError(js, reason.getHandle(js)); + doError(js, kj::mv(reason)); }); algorithms.pulling = maybeRunAlgorithm(js, @@ -601,63 +559,35 @@ void ReadableImpl::pullIfNeeded(jsg::Lock& js, jsg::Ref self) { } template -void ReadableImpl::resolveReadRequest( - ReadResult result, - kj::Maybe maybeRequest) { -// TODO(stream-tee): This goes away because consumers become responsible for resolving reads. - if (maybeRequest != nullptr) { - maybeResolvePromise(maybeRequest, kj::mv(result)); - return; +void ReadableImpl::visitForGc(jsg::GcVisitor& visitor) { + KJ_SWITCH_ONEOF(state) { + KJ_CASE_ONEOF(closed, StreamStates::Closed) {} + KJ_CASE_ONEOF(errored, StreamStates::Errored) { + visitor.visit(errored); + } + KJ_CASE_ONEOF(queue, Queue) { + visitor.visit(queue); + } + } + KJ_IF_MAYBE(pendingCancel, maybePendingCancel) { + visitor.visit(pendingCancel->fulfiller, pendingCancel->promise); } - dequeueReadRequest().resolve(kj::mv(result)); + visitor.visit(algorithms); } template -void ReadableImpl::setup( - jsg::Lock& js, - jsg::Ref self, - UnderlyingSource underlyingSource, - StreamQueuingStrategy queuingStrategy) { - bool isBytes = underlyingSource.type.map([](auto& s) { return s == "bytes"; }).orDefault(false); - -// TODO(stream-tee): the highWaterMark is handled by the queue impl - highWaterMark = queuingStrategy.highWaterMark.orDefault(isBytes ? 0 : 1); - - auto startAlgorithm = kj::mv(underlyingSource.start); - algorithms.pull = kj::mv(underlyingSource.pull); - algorithms.cancel = kj::mv(underlyingSource.cancel); - algorithms.size = kj::mv(queuingStrategy.size); - - auto onSuccess = JSG_VISITABLE_LAMBDA((this, self = self.addRef()), (self), (jsg::Lock& js) { - algorithms.starting = nullptr; - started = true; - pullIfNeeded(js, kj::mv(self)); - }); - - auto onFailure = JSG_VISITABLE_LAMBDA((this,self = self.addRef()), (self), - (jsg::Lock& js, jsg::Value reason) { - algorithms.starting = nullptr; - started = true; - doError(js, reason.getHandle(js)); - }); - - algorithms.starting = maybeRunAlgorithm(js, - startAlgorithm, - kj::mv(onSuccess), - kj::mv(onFailure), - self.addRef()); +kj::Own::Consumer> +ReadableImpl::getConsumer(kj::Maybe::StateListener&> listener) { + auto& queue = state.template get(); + return kj::heap::Consumer>(queue, listener); } template -void ReadableImpl::visitForGc(jsg::GcVisitor& visitor) { - KJ_IF_MAYBE(error, state.tryGet()) { - visitor.visit(*error); +bool ReadableImpl::hasPendingReadRequests() { + KJ_IF_MAYBE(queue, state.template tryGet()) { + return queue->wantsRead(); } - KJ_IF_MAYBE(pendingCancel, maybePendingCancel) { - visitor.visit(pendingCancel->fulfiller, pendingCancel->promise); - } - visitor.visit(algorithms, queue); - visitor.visitAll(readRequests); + return false; } // ====================================================================================== @@ -703,7 +633,7 @@ jsg::Promise WritableImpl::abort( template ssize_t WritableImpl::getDesiredSize() { - return highWaterMark - queue.size(); + return highWaterMark - amountBuffered; } template @@ -717,53 +647,53 @@ void WritableImpl::advanceQueueIfNeeded(jsg::Lock& js, jsg::Ref self return finishErroring(js, kj::mv(self)); } - if (queue.empty()) { - return; - } + if (writeRequests.empty()) { + KJ_IF_MAYBE(req, closeRequest) { + KJ_ASSERT(inFlightClose == nullptr); + KJ_ASSERT_NONNULL(closeRequest); + inFlightClose = kj::mv(closeRequest); - if (queue.frontIsClose()) { - KJ_ASSERT(inFlightClose == nullptr); - KJ_ASSERT_NONNULL(closeRequest); - inFlightClose = kj::mv(closeRequest); - queue.template pop(); - KJ_ASSERT(queue.empty()); - - auto onSuccess = JSG_VISITABLE_LAMBDA((this, self = self.addRef()), (self), (jsg::Lock& js) { - algorithms.closing = nullptr; - finishInFlightClose(js, kj::mv(self)); - }); + auto onSuccess = JSG_VISITABLE_LAMBDA( + (this, self = self.addRef()), (self), (jsg::Lock& js) { + algorithms.closing = nullptr; + finishInFlightClose(js, kj::mv(self)); + }); - auto onFailure = JSG_VISITABLE_LAMBDA((this, self = self.addRef()), (self), - (jsg::Lock& js, jsg::Value reason) { - algorithms.closing = nullptr; - finishInFlightClose(js, kj::mv(self), reason.getHandle(js)); - }); + auto onFailure = JSG_VISITABLE_LAMBDA( + (this, self = self.addRef()), (self), (jsg::Lock& js, jsg::Value reason) { + algorithms.closing = nullptr; + finishInFlightClose(js, kj::mv(self), reason.getHandle(js)); + }); - algorithms.closing = maybeRunAlgorithm(js, - algorithms.close, - kj::mv(onSuccess), - kj::mv(onFailure)); + algorithms.closing = maybeRunAlgorithm(js, + algorithms.close, + kj::mv(onSuccess), + kj::mv(onFailure)); + } return; } - auto& chunk = queue.peek(); - KJ_ASSERT(inFlightWrite == nullptr); - inFlightWrite = dequeueWriteRequest(); - - auto onSuccess = JSG_VISITABLE_LAMBDA((this, self = self.addRef()), (self), (jsg::Lock& js) { + auto req = dequeueWriteRequest(); + auto value = req.value.addRef(js); + auto size = req.size; + inFlightWrite = kj::mv(req); + + auto onSuccess = JSG_VISITABLE_LAMBDA( + (this, self = self.addRef(), size), (self), (jsg::Lock& js) { + amountBuffered -= size; algorithms.writing = nullptr; finishInFlightWrite(js, self.addRef()); KJ_ASSERT(state.template is() || state.template is()); - queue.pop(); if (!isCloseQueuedOrInFlight() && state.template is()) { updateBackpressure(js); } advanceQueueIfNeeded(js, kj::mv(self)); }); - auto onFailure = JSG_VISITABLE_LAMBDA((this, self = self.addRef()), (self), + auto onFailure = JSG_VISITABLE_LAMBDA((this, self = self.addRef(), size), (self), (jsg::Lock& js, jsg::Value reason) { + amountBuffered -= size; algorithms.writing = nullptr; finishInFlightWrite(js, kj::mv(self), reason.getHandle(js)); }); @@ -772,7 +702,7 @@ void WritableImpl::advanceQueueIfNeeded(jsg::Lock& js, jsg::Ref self algorithms.write, kj::mv(onSuccess), kj::mv(onFailure), - chunk.value.getHandle(js), + value.getHandle(js), self.addRef()); } @@ -788,7 +718,6 @@ jsg::Promise WritableImpl::close(jsg::Lock& js, jsg::Ref self) getOwner().maybeResolveReadyPromise(); } - queue.close(); advanceQueueIfNeeded(js, kj::mv(self)); return kj::mv(prp.promise); @@ -807,7 +736,7 @@ void WritableImpl::dealWithRejection( } template -WriteRequest WritableImpl::dequeueWriteRequest() { +typename WritableImpl::WriteRequest WritableImpl::dequeueWriteRequest() { auto write = kj::mv(writeRequests.front()); writeRequests.pop_front(); return kj::mv(write); @@ -821,7 +750,6 @@ void WritableImpl::doClose() { KJ_ASSERT(maybePendingAbort == nullptr); KJ_ASSERT(writeRequests.empty()); state.template init(); - queue.reset(); algorithms.clear(); KJ_IF_MAYBE(theOwner, owner) { @@ -840,7 +768,6 @@ void WritableImpl::doError(jsg::Lock& js, v8::Local reason) { KJ_ASSERT(maybePendingAbort == nullptr); KJ_ASSERT(writeRequests.empty()); state = js.v8Ref(reason); - queue.reset(); algorithms.clear(); KJ_IF_MAYBE(theOwner, owner) { @@ -866,13 +793,12 @@ template void WritableImpl::finishErroring(jsg::Lock& js, jsg::Ref self) { auto erroring = kj::mv(KJ_ASSERT_NONNULL(state.template tryGet())); auto reason = erroring.reason.getHandle(js); - KJ_ASSERT(inFlightWrite == nullptr && inFlightClose == nullptr); + KJ_ASSERT(inFlightWrite == nullptr); + KJ_ASSERT(inFlightClose == nullptr); state.template init(kj::mv(erroring.reason)); - queue.reset(); - while (!writeRequests.empty()) { - dequeueWriteRequest().reject(reason); + dequeueWriteRequest().resolver.reject(reason); } KJ_ASSERT(writeRequests.empty()); @@ -946,15 +872,17 @@ void WritableImpl::finishInFlightWrite( jsg::Lock& js, jsg::Ref self, kj::Maybe> maybeReason) { - KJ_ASSERT_NONNULL(inFlightWrite); + auto& write = KJ_ASSERT_NONNULL(inFlightWrite); KJ_IF_MAYBE(reason, maybeReason) { - maybeRejectPromise(inFlightWrite, *reason); + write.resolver.reject(js, *reason); + inFlightWrite = nullptr; KJ_ASSERT(state.template is() || state.template is()); return dealWithRejection(js, kj::mv(self), *reason); } - maybeResolvePromise(inFlightWrite); + write.resolver.resolve(); + inFlightWrite = nullptr; } template @@ -1054,16 +982,20 @@ jsg::Promise WritableImpl::write( jsg::Lock& js, jsg::Ref self, v8::Local value) { - size_t size = jscontroller::getChunkSize( - js, - algorithms.size, - value, - [&](jsg::Lock& js, v8::Local error) { - if (state.template is()) { - algorithms.clear(); - startErroring(js, self.addRef(), error); + + size_t size = 1; + KJ_IF_MAYBE(sizeFunc, algorithms.size) { + kj::Maybe failure; + js.tryCatch([&] { + size = (*sizeFunc)(js, value); + }, [&](jsg::Value exception) { + startErroring(js, self.addRef(), exception.getHandle(js)); + failure = kj::mv(exception); + }); + KJ_IF_MAYBE(exception, failure) { + return js.rejectedPromise(kj::mv(*exception)); } - }).orDefault(1); + } KJ_IF_MAYBE(error, state.tryGet()) { return js.rejectedPromise(error->addRef(js)); @@ -1080,12 +1012,12 @@ jsg::Promise WritableImpl::write( KJ_ASSERT(state.template is()); auto prp = js.newPromiseAndResolver(); - writeRequests.push_back(kj::mv(prp.resolver)); - - queue.push(ValueQueueEntry { + writeRequests.push_back(WriteRequest { + .resolver = kj::mv(prp.resolver), .value = js.v8Ref(value), - .size = size + .size = size, }); + amountBuffered += size; updateBackpressure(js); advanceQueueIfNeeded(js, kj::mv(self)); @@ -1108,1283 +1040,702 @@ void WritableImpl::visitForGc(jsg::GcVisitor &visitor) { inFlightClose, closeRequest, algorithms, - queue, signal, maybePendingAbort); visitor.visitAll(writeRequests); } } // namespace jscontroller -// ======================================================================================= +// ====================================================================================== -ReadableStreamDefaultController::ReadableStreamDefaultController(ReaderOwner& owner) - : impl(owner) {} -// TODO(stream-tee): The controller will no longer have any notion of a single owner. As long -// as there are consumers known to the queue, the controller will be kept alive. State changes -// are communicated via the consumer. +namespace { +template +struct ReadableState { + jsg::Ref controller; + kj::Maybe> owner; + kj::Own consumer; -void ReadableStreamDefaultController::setOwner(kj::Maybe owner) { - impl.setOwner(owner); -} + ReadableState( + jsg::Ref controller, auto owner, auto stateListener) + : controller(kj::mv(controller)), + owner(owner), + consumer(this->controller->getConsumer(stateListener)) {} -jsg::Promise ReadableStreamDefaultController::cancel( - jsg::Lock& js, - jsg::Optional> maybeReason) { -// TODO(stream-tee): This cancel is triggered by the ReadableStreamJsController via the -// ValueReadable or ByteReadable. Some of the functionality needs to be moved into those. -// For instance, the individual consumer will be responsible for resolving its own -// collection of read requests. The cancel algorithm will only be invoked when the -// last consumer known to the queue is canceling. - return impl.cancel(js, JSG_THIS, maybeReason.orDefault(js.v8Undefined())); -} + ReadableState(jsg::Ref controller, auto owner, kj::Own consumer) + : controller(kj::mv(controller)), + owner(owner), + consumer(kj::mv(consumer)) {} -void ReadableStreamDefaultController::close(jsg::Lock& js) { -// TODO(stream-tee): This close is triggered by user-call. It must result in the queue -// being closed, which in turn will communicate the close to each of the consumers. -// It will be the responsibility of the consumers to cancel their pending pull intos. - JSG_REQUIRE(impl.canCloseOrEnqueue(), - TypeError, - "This ReadableStreamDefaultController is closed."); - impl.closeRequested = true; - if (impl.queue.empty()) { - impl.doClose(js); + void setOwner(auto newOwner) { + owner = newOwner; } -} - -void ReadableStreamDefaultController::doCancel(jsg::Lock& js, v8::Local reason) { -// TODO(stream-tee): Revisit this - impl.doCancel(js, JSG_THIS, reason); -} - -void ReadableStreamDefaultController::enqueue( - jsg::Lock& js, - jsg::Optional> chunk) { - JSG_REQUIRE(impl.canCloseOrEnqueue(), - TypeError, - "This ReadableStreamDefaultController is closed."); - doEnqueue(js, chunk); -} -void ReadableStreamDefaultController::doEnqueue( - jsg::Lock& js, - jsg::Optional> chunk) { -// TODO(stream-tee): Do we need the separate doEnqueue? The implementation here changes a bit. -// the enqueue will push into the ValueQueue and the ValueQueue::Consumers will handle resolving -// the reads. - KJ_ASSERT(impl.canCloseOrEnqueue()); + bool hasPendingReadRequests() { + return consumer->hasReadRequests(); + } - auto value = chunk.orDefault(js.v8Undefined()); + jsg::Promise cancel(jsg::Lock& js, jsg::Optional> maybeReason) { + consumer->cancel(js, maybeReason); + return controller->cancel(js, kj::mv(maybeReason)); + } - KJ_DEFER(impl.pullIfNeeded(js, JSG_THIS)); - if (!impl.getOwner().isLocked() || impl.readRequests.empty()) { - KJ_IF_MAYBE(size, jscontroller::getChunkSize( - js, - impl.algorithms.size, - value, - [&](jsg::Lock& js, v8::Local error) { impl.doError(js, error); })) { - impl.queue.push(jscontroller::ValueQueueEntry { js.v8Ref(value), *size }); + void consumerClose() { + KJ_IF_MAYBE(o, owner) { + KJ_SWITCH_ONEOF(*o) { + KJ_CASE_ONEOF(controller, ReadableStreamJsController*) { + return controller->doClose(); + } + KJ_CASE_ONEOF(source, ReadableStreamJsSource*) { + return source->doClose(); + } + } + KJ_UNREACHABLE; } - return; } - KJ_ASSERT(impl.queue.empty()); - impl.resolveReadRequest( - ReadResult { - .value = js.v8Ref(value), - .done = false, - }); -} - -void ReadableStreamDefaultController::error(jsg::Lock& js, v8::Local reason) { -// TODO(stream-tee): Called by user-call. We need to notify the queue of the error and notify -// all of the consumers about the error. - if (impl.state.is()) { - impl.doError(js, reason); + void consumerError(jsg::Lock& js, jsg::Value reason) { + KJ_IF_MAYBE(o, owner) { + KJ_SWITCH_ONEOF(*o) { + KJ_CASE_ONEOF(controller, ReadableStreamJsController*) { + return controller->doError(js, reason.getHandle(js)); + } + KJ_CASE_ONEOF(source, ReadableStreamJsSource*) { + return source->doError(js, reason.getHandle(js)); + } + } + KJ_UNREACHABLE; + } } -} -kj::Maybe ReadableStreamDefaultController::getDesiredSize() { - return impl.getDesiredSize(); -} + void consumerWantsData(jsg::Lock& js) { + controller->pull(js); + } -bool ReadableStreamDefaultController::hasPendingReadRequests() { -// TODO(stream-tee): Needs to be modified such that if any consumer has pending reads, this -// returns true. - return !impl.readRequests.empty(); -} - -void ReadableStreamDefaultController::pull(jsg::Lock& js, ReadRequest readRequest) { -// TODO(stream-tee): Needs to be reimplemented in terms of the consumer. Essentially, the consumer -// receives the read request. If the consumer doesn't have the data in it's own queue to serve -// the request, it will ask the queue to get it. If there's no data in the queue, then we need -// to pull from the underlying source. - - // This should only be called if the stream is readable - KJ_ASSERT(impl.state.is()); - if (!impl.queue.empty()) { - // Here the entry should always be a ValueQueueEntry. - auto entry = impl.queue.pop(); - if (impl.closeRequested && impl.queue.empty()) { - impl.doClose(js); - } else { - impl.pullIfNeeded(js, JSG_THIS); - } - impl.resolveReadRequest( - ReadResult { - .value = kj::mv(entry.value), - .done = false, - }, - kj::mv(readRequest)); - return; + void visitForGc(jsg::GcVisitor& visitor) { + visitor.visit(*consumer); } - impl.readRequests.push_back(kj::mv(readRequest)); - impl.pullIfNeeded(js, JSG_THIS); -} -jsg::Promise ReadableStreamDefaultController::read(jsg::Lock& js) { + ReadableState cloneWithNewOwner(jsg::Lock& js, auto owner, auto stateListener) { + return ReadableState(controller.addRef(), owner, consumer->clone(js, stateListener)); + } - if (impl.state.is()) { - return js.resolvedPromise(ReadResult { .done = true }); + kj::Maybe getDesiredSize() { + return controller->getDesiredSize(); } - KJ_IF_MAYBE(errored, impl.state.tryGet()) { - return js.rejectedPromise(errored->addRef(js)); + bool canCloseOrEnqueue() { + return controller->canCloseOrEnqueue(); } - auto prp = js.newPromiseAndResolver(); - pull(js, kj::mv(prp.resolver)); - return kj::mv(prp.promise); -} + jsg::Ref getControllerRef() { + return controller.addRef(); + } +}; +} // namespace -void ReadableStreamDefaultController::setup( - jsg::Lock& js, - UnderlyingSource underlyingSource, - StreamQueuingStrategy queuingStrategy) { - impl.setup(js, JSG_THIS, kj::mv(underlyingSource), kj::mv(queuingStrategy)); -} +struct ValueReadable final: public api::ValueQueue::ConsumerImpl::StateListener, + public kj::Refcounted { + using State = ReadableState; + kj::Maybe state; -// ====================================================================================== + ValueReadable(jsg::Ref controller, auto owner) + : state(State(kj::mv(controller), owner, this)) {} -// TODO(stream-tee): The ReadableStreamBYOBRequest needs to be modified to operate in -// terms of the ByteQueue::Consumer model. + ValueReadable(jsg::Lock& js, auto owner, ValueReadable& other) + : state(KJ_ASSERT_NONNULL(other.state).cloneWithNewOwner(js, owner, this)) {} -ReadableStreamBYOBRequest::Impl::Impl( - jsg::V8Ref view, - jsg::Ref controller, - size_t atLeast) - : view(kj::mv(view)), - controller(kj::mv(controller)), - atLeast(atLeast) {} + KJ_DISALLOW_COPY(ValueReadable); -void ReadableStreamBYOBRequest::visitForGc(jsg::GcVisitor& visitor) { - KJ_IF_MAYBE(impl, maybeImpl) { - visitor.visit(impl->view, impl->controller); - } + void visitForGc(jsg::GcVisitor& visitor) { + visitor.visit(state); } -ReadableStreamBYOBRequest::ReadableStreamBYOBRequest( - jsg::V8Ref view, - jsg::Ref controller, - size_t atLeast) - : maybeImpl(Impl(kj::mv(view), kj::mv(controller), atLeast)) {} - -kj::Maybe ReadableStreamBYOBRequest::getAtLeast() { - KJ_IF_MAYBE(impl, maybeImpl) { - return impl->atLeast; + bool hasPendingReadRequests() { + return state.map([](State& state) { return state.hasPendingReadRequests(); }).orDefault(false); } - return nullptr; -} -kj::Maybe> ReadableStreamBYOBRequest::getView(jsg::Lock& js) { - KJ_IF_MAYBE(impl, maybeImpl) { - return impl->view.addRef(js); + void setOwner(ReadableStreamJsSource* newOwner) { + KJ_IF_MAYBE(s, state) { s->setOwner(newOwner); } } - return nullptr; -} -void ReadableStreamBYOBRequest::invalidate(jsg::Lock& js) { - KJ_IF_MAYBE(impl, maybeImpl) { - // If the user code happened to have retained a reference to the view or - // the buffer, we need to detach it so that those references cannot be used - // to modify or observe modifications. - impl->view.getHandle(js)->Buffer()->Detach(); - impl->controller->maybeByobRequest = nullptr; + kj::Own clone(jsg::Lock& js, ReadableStreamJsController* owner) { + // A single ReadableStreamDefaultController can have multiple consumers. + // When the ValueReadable constructor is used, the new consumer is added + // and starts to receive new data that becomes enqueued. When clone + // is used, any state currently held by this consumer is copied to the + // new consumer. + return kj::refcounted(js, owner, *this); } - maybeImpl = nullptr; -} -void ReadableStreamBYOBRequest::respond(jsg::Lock& js, int bytesWritten) { - auto& impl = JSG_REQUIRE_NONNULL(maybeImpl, - TypeError, - "This ReadableStreamBYOBRequest has been invalidated."); - JSG_REQUIRE(!impl.controller->pendingPullIntos.empty(), - TypeError, - "There are no pending BYOB read requests."); + jsg::Promise read(jsg::Lock& js) { + KJ_IF_MAYBE(s, state) { + // It's possible for the controller to be closed synchronously while the + // read operation is executing. In that case, we want to make sure we keep + // a reference so it'll survice at least long enough for the read method + // to complete. + auto self KJ_UNUSED = kj::addRef(*this); - if (!impl.controller->isReadable()) { - JSG_REQUIRE(bytesWritten == 0, - TypeError, - "The bytesWritten must be zero after the stream is closed."); - } else { - JSG_REQUIRE(bytesWritten > 0, - TypeError, - "The bytesWritten must be more than zero while the stream is open."); - } + auto prp = js.newPromiseAndResolver(); + s->consumer->read(js, ValueQueue::ReadRequest { + .resolver = kj::mv(prp.resolver), + }); + return kj::mv(prp.promise); + } - auto& pullInto = impl.controller->pendingPullIntos.front(); - JSG_REQUIRE(pullInto.filled + bytesWritten <= pullInto.store.size(), - RangeError, "Too many bytes written."); + // We are canceled! There's nothing to do. + return js.resolvedPromise(ReadResult { .done = true }); + } - // Spec says to detach pullInto's buffer, but it's just a backing store - // and we'll be invalidating the BYOBRequest in the next step so skip that... - impl.controller->respondInternal(js, bytesWritten); -} + jsg::Promise cancel(jsg::Lock& js, jsg::Optional> maybeReason) { + // When a ReadableStream is canceled, the expected behavior is that the underlying + // controller is notified and the cancel algorithm on the underlying source is + // called. When there are multiple ReadableStreams sharing consumption of a + // controller, however, it should act as a shared pointer of sorts, canceling + // the underlying controller only when the last reader is canceled. + // Here, we rely on the controller implementing the correct behavior since it owns + // the queue that knows about all of the attached consumers. + KJ_IF_MAYBE(s, state) { + KJ_DEFER({ + // Clear the references to the controller, free the consumer, and the + // owner state once this scope exits. This ValueReadable will no longer + // be usable once this is done. + auto released KJ_UNUSED = kj::mv(*s); + }); -void ReadableStreamBYOBRequest::respondWithNewView(jsg::Lock& js, jsg::BufferSource view) { - auto& impl = JSG_REQUIRE_NONNULL(maybeImpl, - TypeError, - "This ReadableStreamBYOBRequest has been invalidated."); - JSG_REQUIRE(!impl.controller->pendingPullIntos.empty(), - TypeError, - "There are no pending BYOB read requests."); + return s->cancel(js, kj::mv(maybeReason)); + } - if (!impl.controller->isReadable()) { - JSG_REQUIRE(view.size() == 0, - TypeError, - "The view byte length must be zero after the stream is closed."); - } else { - JSG_REQUIRE(view.size() > 0, - TypeError, - "The view byte length must be more than zero while the stream is open."); + return js.resolvedPromise(); } - impl.controller->respondInternal(js, impl.controller->updatePullInto(js, kj::mv(view))); -} + void onConsumerClose(jsg::Lock& js) override { + // Called by the consumer when a state change to closed happens. + // We need to notify the owner + KJ_IF_MAYBE(s, state) { s->consumerClose(); } + } -// ====================================================================================== + void onConsumerError(jsg::Lock& js, jsg::Value reason) override { + // Called by the consumer when a state change to errored happens. + // We need to noify the owner + KJ_IF_MAYBE(s, state) { s->consumerError(js, kj::mv(reason)); } + } -ReadableByteStreamController::ReadableByteStreamController(ReaderOwner& owner) - : impl(owner) { -// TODO(stream-tee): The notion of a single owner for the controller goes away. The controller -// will have one or more consumers. As long as there are consumers, the controller will be -// available. -} + void onConsumerWantsData(jsg::Lock& js) override { + // Called by the consumer when it has a queued pending read and needs + // data to be provided to fulfill it. We need to notify the controller + // to initiate pulling to provide the data. + KJ_IF_MAYBE(s, state) { s->consumerWantsData(js); } + } -kj::Maybe ReadableByteStreamController::getDesiredSize() { - return impl.getDesiredSize(); -} + kj::Maybe getDesiredSize() { + KJ_IF_MAYBE(s, state) { return s->getDesiredSize(); } + return nullptr; + } -jsg::Promise ReadableByteStreamController::cancel( - jsg::Lock& js, - jsg::Optional> maybeReason) { -// TODO(stream-tee): This cancel is triggered by the ReadableStreamJsController via the -// ValueReadable or ByteReadable. Some of the functionality needs to be moved into those. -// For instance, the individual consumer will be responsible for resolving its own -// collection of read requests. The cancel algorithm will only be invoked when the -// last consumer known to the queue is canceling. - pendingPullIntos.clear(); - while (!impl.readRequests.empty()) { - impl.dequeueReadRequest().resolve(ReadResult { .done = true }); + bool canCloseOrEnqueue() { + return state.map([](State& state) { return state.canCloseOrEnqueue(); }).orDefault(false); } - return impl.cancel(js, JSG_THIS, maybeReason.orDefault(js.v8Undefined())); -} -void ReadableByteStreamController::close(jsg::Lock& js) { -// TODO(stream-tee): This close is triggered by user-call. It must result in the queue -// being closed, which in turn will communicate the close to each of the consumers. -// It will be the responsibility of the consumers to cancel their pending pull intos. - JSG_REQUIRE(impl.canCloseOrEnqueue(), TypeError, - "This ReadableByteStreamController is closed."); - if (!impl.queue.empty()) { - impl.closeRequested = true; - return; + kj::Maybe> getControllerRef() { + return state.map([](State& state) { return state.getControllerRef(); }); } - if (!pendingPullIntos.empty()) { - auto& pullInto = pendingPullIntos.front(); - if (pullInto.filled > 0) { - auto error = js.v8TypeError( - "This ReadablebyteStreamController was closed with a partial BYOB read"_kj); - impl.doError(js, error); - jsg::throwTunneledException(js.v8Isolate, error); +}; + +struct ByteReadable final: public api::ByteQueue::ConsumerImpl::StateListener, + public kj::Refcounted { + using State = ReadableState; + kj::Maybe state; + int autoAllocateChunkSize; + + ByteReadable( + jsg::Ref controller, + auto owner, + int autoAllocateChunkSize) + : state(State(kj::mv(controller), owner, this)), + autoAllocateChunkSize(autoAllocateChunkSize) {} + + ByteReadable(jsg::Lock& js, auto owner, ByteReadable& other) + : state(KJ_ASSERT_NONNULL(other.state).cloneWithNewOwner(js, owner, this)), + autoAllocateChunkSize(other.autoAllocateChunkSize) {} + + KJ_DISALLOW_COPY(ByteReadable); + + void visitForGc(jsg::GcVisitor& visitor) { + visitor.visit(state); + } + + bool hasPendingReadRequests() { + return state.map([](State& state) { return state.hasPendingReadRequests(); }).orDefault(false); + } + + void setOwner(ReadableStreamJsSource* newOwner) { + KJ_IF_MAYBE(s, state) { s->setOwner(newOwner); } + } + + kj::Own clone(jsg::Lock& js, ReadableStreamJsController* owner) { + // A single ReadableByteStreamController can have multiple consumers. + // When the ByteReadable constructor is used, the new consumer is added + // and starts to receive new data that becomes enqueued. When clone + // is used, any state currently held by this consumer is copied to the + // new consumer. + return kj::refcounted(js, owner, *this); + } + + jsg::Promise read( + jsg::Lock& js, + kj::Maybe byobOptions) { + KJ_IF_MAYBE(s, state) { + // It's possible for the controller to be closed synchronously while the + // read operation is executing. In that case, we want to make sure we keep + // a reference so it'll survice at least long enough for the read method + // to complete. + auto self KJ_UNUSED = kj::addRef(*this); + + auto prp = js.newPromiseAndResolver(); + + KJ_IF_MAYBE(byob, byobOptions) { + jsg::BufferSource source(js, byob->bufferView.getHandle(js)); + // If atLeast is not given, then by default it is the element size of the view + // that we were given. If atLeast is given, we make sure that it is aligned + // with the element size. No matter what, atLeast cannot be less than 1. + auto atLeast = kj::max(source.getElementSize(), byob->atLeast.orDefault(1)); + atLeast = kj::max(1, atLeast - (atLeast % source.getElementSize())); + s->consumer->read(js, ByteQueue::ReadRequest { + .resolver = kj::mv(prp.resolver), + .pullInto { + .store = source.detach(js), + .atLeast = atLeast, + .type = ByteQueue::ReadRequest::Type::BYOB, + }, + }); + } else { + s->consumer->read(js, ByteQueue::ReadRequest { + .resolver = kj::mv(prp.resolver), + .pullInto { + .store = jsg::BackingStore::alloc(js, autoAllocateChunkSize), + .type = ByteQueue::ReadRequest::Type::BYOB, + }, + }); + } + + return kj::mv(prp.promise); } - } - impl.doClose(js); -} -void ReadableByteStreamController::commitPullInto(jsg::Lock& js, PendingPullInto pullInto) { -// TODO(stream-tee): Responsibility here moves into the ByteQueue::Consumer implementation. - bool done = false; - if (impl.state.is()) { - KJ_ASSERT(pullInto.filled == 0); - done = true; - } - pullInto.store.trim(pullInto.store.size() - pullInto.filled); - impl.resolveReadRequest( - ReadResult { - .value = js.v8Ref(pullInto.store.createHandle(js)), - .done = done, + // We are canceled! There's nothing else to do. + KJ_IF_MAYBE(byob, byobOptions) { + // If a BYOB buffer was given, we need to give it back wrapped in a TypedArray + // whose size is set to zero. + jsg::BufferSource source(js, byob->bufferView.getHandle(js)); + auto store = source.detach(js); + store.consume(store.size()); + return js.resolvedPromise(ReadResult { + .value = js.v8Ref(store.createHandle(js)), + .done = true, }); -} - -ReadableByteStreamController::PendingPullInto -ReadableByteStreamController::dequeuePendingPullInto() { -// TODO(stream-tee): Responsibility here moves into the ByteQueue::Consumer implementation. - KJ_ASSERT(!pendingPullIntos.empty()); - auto pullInto = kj::mv(pendingPullIntos.front()); - pendingPullIntos.pop_front(); - return kj::mv(pullInto); -} + } else { + return js.resolvedPromise(ReadResult { .done = true }); + } + } -void ReadableByteStreamController::doCancel(jsg::Lock& js, v8::Local reason) { -// TODO(stream-tee): Re-evaluate... - impl.doCancel(js, JSG_THIS, reason); -} + jsg::Promise cancel(jsg::Lock& js, jsg::Optional> maybeReason) { + // When a ReadableStream is canceled, the expected behavior is that the underlying + // controller is notified and the cancel algorithm on the underlying source is + // called. When there are multiple ReadableStreams sharing consumption of a + // controller, however, it should act as a shared pointer of sorts, canceling + // the underlying controller only when the last reader is canceled. + // Here, we rely on the controller implementing the correct behavior since it owns + // the queue that knows about all of the attached consumers. + KJ_IF_MAYBE(s, state) { + KJ_DEFER({ + // Clear the references to the controller, free the consumer, and the + // owner state once this scope exits. This ByteReadable will no longer + // be usable once this is done. + auto released KJ_UNUSED = kj::mv(*s); + }); -void ReadableByteStreamController::enqueue(jsg::Lock& js, jsg::BufferSource chunk) { -// TODO(stream-tee): This is called by user-call. It must be modified to push data into the queue, -// which will trigger the cascade of that data into the consumers, which will handle the majority -// of the handling here. - JSG_REQUIRE(chunk.size() > 0, TypeError, "Cannot enqueue a zero-length ArrayBuffer."); - JSG_REQUIRE(chunk.canDetach(js), TypeError, - "The provided ArrayBuffer must be detachable."); - JSG_REQUIRE(impl.canCloseOrEnqueue(), TypeError, "This ReadableByteStreamController is closed."); + return s->cancel(js, kj::mv(maybeReason)); + } - auto backing = chunk.detach(js); + return js.resolvedPromise(); + } - KJ_IF_MAYBE(byobRequest, maybeByobRequest) { - (*byobRequest)->invalidate(js); + void onConsumerClose(jsg::Lock& js) override { + KJ_IF_MAYBE(s, state) { s->consumerClose(); } } - const auto enqueueChunk = [&] { - impl.queue.push(jscontroller::ByteQueueEntry { .store = kj::mv(backing) }); - }; + void onConsumerError(jsg::Lock& js, jsg::Value reason) override { + KJ_IF_MAYBE(s, state) { s->consumerError(js, kj::mv(reason)); }; + } - KJ_DEFER(impl.pullIfNeeded(js, JSG_THIS)); - if (!impl.getOwner().isLocked() || impl.readRequests.empty()) { - return enqueueChunk(); + void onConsumerWantsData(jsg::Lock& js) override { + // Called by the consumer when it has a queued pending read and needs + // data to be provided to fulfill it. We need to notify the controller + // to initiate pulling to provide the data. + KJ_IF_MAYBE(s, state) { s->consumerWantsData(js); } } - if (impl.getOwner().isLockedReaderByteOriented()) { - enqueueChunk(); - pullIntoUsingQueue(js); - } else { - KJ_ASSERT(impl.queue.empty()); - if (!pendingPullIntos.empty()) { - auto pending = dequeuePendingPullInto(); - KJ_ASSERT(pending.type == PendingPullInto::Type::DEFAULT); - } - impl.resolveReadRequest( - ReadResult { - .value = js.v8Ref(backing.getTypedView().createHandle(js)), - .done = false, - }); + kj::Maybe getDesiredSize() { + KJ_IF_MAYBE(s, state) { return s->getDesiredSize(); } + return nullptr; } -} -void ReadableByteStreamController::error(jsg::Lock& js, v8::Local reason) { -// TODO(stream-tee): This is called by the user-call. It must error the queue and communicate to -// each of the consumers that the readable stream is immediately shutting down because of error. - if (impl.state.is()) { - impl.doError(js, reason); - } -} - -bool ReadableByteStreamController::fillPullInto(PendingPullInto& pullInto) { -// TODO(stream-tee): Responsibility here moves into the ByteQueue::Consumer implementation. - auto elementSize = pullInto.store.getElementSize(); - auto currentAlignedBytes = pullInto.filled - (pullInto.filled % elementSize); - auto maxBytesToCopy = kj::min(impl.queue.size(), pullInto.store.size() - pullInto.filled); - auto maxBytesFilled = pullInto.filled + maxBytesToCopy; - auto maxAlignedBytes = maxBytesFilled - (maxBytesFilled % elementSize); - auto totalBytesToCopyRemaining = maxBytesToCopy; - bool ready = false; - - if (maxAlignedBytes > currentAlignedBytes) { - totalBytesToCopyRemaining = maxAlignedBytes - pullInto.filled; - ready = true; - } - - auto destination = pullInto.store.asArrayPtr().begin(); - - while (totalBytesToCopyRemaining > 0) { - // The head will always be a ByteQueueEntry here - auto& head = impl.queue.peek(); - auto bytesToCopy = kj::min(totalBytesToCopyRemaining, head.store.size()); - memcpy(destination, head.store.asArrayPtr().begin(), bytesToCopy); - if (head.store.size() == bytesToCopy) { - auto removeHead = kj::mv(head); - impl.queue.pop(); - } else { - head.store.consume(bytesToCopy); - impl.queue.dec(bytesToCopy); - } - KJ_ASSERT(maybeByobRequest == nullptr); - pullInto.filled += bytesToCopy; - totalBytesToCopyRemaining -= bytesToCopy; - destination += bytesToCopy; + bool canCloseOrEnqueue() { + return state.map([](State& state) { return state.canCloseOrEnqueue(); }).orDefault(false); } - if (!ready) { - KJ_ASSERT(impl.queue.empty()); - KJ_ASSERT(pullInto.filled > 0); - KJ_ASSERT(pullInto.filled < elementSize); + kj::Maybe> getControllerRef() { + return state.map([](State& state) { return state.getControllerRef(); }); } +}; - return ready; -} +// ======================================================================================= -kj::Maybe> ReadableByteStreamController::getByobRequest( - jsg::Lock& js) { -// TODO(stream-tee): The ReadableStreamBYOBRequest mechanism needs to be reworked around -// ByteQueue::Consumer's similar concept. - JSG_REQUIRE(impl.state.is(), - TypeError, - "This ReadableByteStreamController has been closed."); - if (maybeByobRequest == nullptr && !pendingPullIntos.empty()) { - auto& pullInto = pendingPullIntos.front(); - auto view = pullInto.store.getTypedView(); - view.consume(pullInto.filled); - maybeByobRequest = - jsg::alloc( - js.v8Ref(view.createHandle(js).As()), - JSG_THIS, - pullInto.atLeast); - } - return kj::mv(maybeByobRequest); +ReadableStreamDefaultController::ReadableStreamDefaultController( + UnderlyingSource underlyingSource, + StreamQueuingStrategy queuingStrategy) + : impl(kj::mv(underlyingSource), kj::mv(queuingStrategy)) {} + +void ReadableStreamDefaultController::start(jsg::Lock& js) { + impl.start(js, JSG_THIS); } -bool ReadableByteStreamController::hasPendingReadRequests() { -// TODO(stream-tee): Needs to be modified such that if any known consumer has a pending -// read request this returns true. The controller itself will no longer track read requests -// itself. - return !impl.readRequests.empty(); +bool ReadableStreamDefaultController::canCloseOrEnqueue() { + return impl.canCloseOrEnqueue(); } -bool ReadableByteStreamController::isReadable() const { - return impl.state.is(); +bool ReadableStreamDefaultController::hasBackpressure() { + return !impl.shouldCallPull(); } -void ReadableByteStreamController::pullIntoUsingQueue(jsg::Lock& js) { -// TODO(stream-tee): Responsibility here moves into the ByteQueue::Consumer implementation. - KJ_ASSERT(!impl.closeRequested); - while (!pendingPullIntos.empty() && !impl.queue.empty()) { - auto& pullInto = pendingPullIntos.front(); - if (fillPullInto(pullInto)) { - commitPullInto(js, dequeuePendingPullInto()); - } - } +kj::Maybe ReadableStreamDefaultController::getDesiredSize() { + return impl.getDesiredSize(); } -void ReadableByteStreamController::pull(jsg::Lock& js, ReadRequest readRequest) { -// TODO(stream-tee): Triggers the pull algorithm to be called on this controller. -// We will need to rework the impl.queue.empty() piece below. Is it even still -// relevant with the new queue model? +bool ReadableStreamDefaultController::hasPendingReadRequests() { + return impl.hasPendingReadRequests(); +} - // This should only ever be called if the stream is readable - KJ_ASSERT(impl.state.is()); - if (!impl.queue.empty()) { - KJ_ASSERT(impl.readRequests.empty()); - auto entry = impl.queue.pop(); - queueDrain(js); - impl.resolveReadRequest( - ReadResult { - .value = js.v8Ref(entry.store.getTypedView().createHandle(js)), - .done = false, - }, - kj::mv(readRequest)); - return; - } - // Per the spec, we're only supposed to follow the next step if autoAllocateChunkSize - // is enabled. We *always* support autoAllocateChunkSize. If the user has not specified - // the size explicitly, we'll use a default, so autoAllocateChunkSize is never undefined. - pendingPullIntos.push_back(PendingPullInto { - .store = jsg::BackingStore::alloc(js, autoAllocateChunkSize), - .filled = 0, - .atLeast = 1, - .type = PendingPullInto::Type::DEFAULT - }); +void ReadableStreamDefaultController::visitForGc(jsg::GcVisitor& visitor) { + visitor.visit(impl); +} - impl.readRequests.push_back(kj::mv(readRequest)); - impl.pullIfNeeded(js, JSG_THIS); +jsg::Promise ReadableStreamDefaultController::cancel( + jsg::Lock& js, + jsg::Optional> maybeReason) { + return impl.cancel(js, JSG_THIS, maybeReason.orDefault([&] { return js.v8Undefined(); })); } -void ReadableByteStreamController::queueDrain(jsg::Lock& js) { -// TODO(stream-tee): Need to re-evaluate this. Essentially, this is a graceful -// close. If close has been requested and the queue is empty, we signal to all -// consumers that we're done and they should disconnect, otherwise we try -// pulling more data. - if (impl.queue.size() == 0 && impl.closeRequested) { - return impl.doClose(js); - } - impl.pullIfNeeded(js, JSG_THIS); +void ReadableStreamDefaultController::close(jsg::Lock& js) { + impl.close(js); } -jsg::Promise ReadableByteStreamController::read( +void ReadableStreamDefaultController::enqueue( jsg::Lock& js, - kj::Maybe maybeByobOptions) { -// TODO(stream-tee): This likely goes away. It will be up to the ByteQueue::Consumer -// to handle this functionality. - if (impl.state.is()) { - KJ_IF_MAYBE(byobOptions, maybeByobOptions) { - // We're going to return an empty ArrayBufferView using the same underlying buffer but with - // the length set to 0, and with the same type as the one we were given. - auto source = jsg::BufferSource(js, byobOptions->bufferView.getHandle(js)); - auto store = source.detach(js); - store.consume(store.size()); // Ensures that our return is zero-length. - - return js.resolvedPromise(ReadResult { - .value = js.v8Ref(store.createHandle(js)), - .done = true, - }); - } + jsg::Optional> chunk) { + auto value = chunk.orDefault(js.v8Undefined()); - // We weren't given an ArrayBufferView to fill but we still want to return an empty one here. - return js.resolvedPromise(ReadResult { - .value = js.v8Ref(v8::Uint8Array::New(v8::ArrayBuffer::New(js.v8Isolate, 0), 0, 0) - .As()), - .done = true, + size_t size = 1; + bool errored = false; + KJ_IF_MAYBE(sizeFunc, impl.algorithms.size) { + js.tryCatch([&] { + size = (*sizeFunc)(js, value); + }, [&](jsg::Value exception) { + impl.doError(js, kj::mv(exception)); + errored = true; }); } - KJ_IF_MAYBE(errored, impl.state.tryGet()) { - return js.rejectedPromise(errored->addRef(js)); - } - - auto readRequest = js.newPromiseAndResolver(); - - KJ_IF_MAYBE(byobOptions, maybeByobOptions) { - auto source = jsg::BufferSource(js, byobOptions->bufferView.getHandle(js)); - auto store = source.detach(js); - auto pullInto = PendingPullInto { - .store = kj::mv(store), - .filled = 0, - .atLeast = byobOptions->atLeast.orDefault(1), - .type = PendingPullInto::Type::BYOB, - }; - - if (!pendingPullIntos.empty()) { - pendingPullIntos.push_back(kj::mv(pullInto)); - impl.readRequests.push_back(kj::mv(readRequest.resolver)); - return kj::mv(readRequest.promise); - } - - if (!impl.queue.empty()) { - if (fillPullInto(pullInto)) { - pullInto.store.trim(pullInto.store.size() - pullInto.filled); - v8::Local view = pullInto.store.createHandle(js); - queueDrain(js); - readRequest.resolver.resolve(ReadResult { - .value = js.v8Ref(view), - .done = false, - }); - return kj::mv(readRequest.promise); - } - - if (impl.closeRequested) { - auto error = js.v8TypeError("This ReadableStream is closed."_kj); - impl.doError(js, error); - readRequest.resolver.reject(error); - return kj::mv(readRequest.promise); - } - } - - pendingPullIntos.push_back(kj::mv(pullInto)); - impl.readRequests.push_back(kj::mv(readRequest.resolver)); - - impl.pullIfNeeded(js, JSG_THIS); - return kj::mv(readRequest.promise); + if (!errored) { + impl.enqueue(js, kj::refcounted(js.v8Ref(value), size), JSG_THIS); } - - // Using the default reader! - pull(js, kj::mv(readRequest.resolver)); - return kj::mv(readRequest.promise); } -void ReadableByteStreamController::respondInternal(jsg::Lock& js, size_t bytesWritten) { -// TODO(stream-tee): This likely goes away and is handled by the ByteQueue::Consumer. - auto& pullInto = pendingPullIntos.front(); - KJ_DEFER(KJ_IF_MAYBE(byobRequest, maybeByobRequest) { - (*byobRequest)->invalidate(js); - }); - - if (impl.state.is()) { - KJ_ASSERT(bytesWritten == 0); - KJ_ASSERT(pullInto.filled == 0); - if (impl.getOwner().isLockedReaderByteOriented()) { - while (!impl.readRequests.empty()) { - commitPullInto(js, dequeuePendingPullInto()); - } - } - } else { - auto elementSize = pullInto.store.getElementSize(); - KJ_ASSERT(pullInto.filled + bytesWritten <= pullInto.store.size()); - KJ_ASSERT(pendingPullIntos.empty() || &pendingPullIntos.front() == &pullInto); - KJ_ASSERT(maybeByobRequest == nullptr); - pullInto.filled += bytesWritten; - if (pullInto.filled < elementSize) { - return; - } - pullInto = dequeuePendingPullInto(); - auto remainderSize = pullInto.filled % elementSize; - if (remainderSize > 0) { - auto end = pullInto.store.getOffset() + pullInto.filled; - auto backing = jsg::BackingStore::alloc(js, remainderSize); - memcpy( - backing.asArrayPtr().begin(), - pullInto.store.asArrayPtr().begin() + (end - remainderSize), - remainderSize); - impl.queue.push(jscontroller::ByteQueueEntry { .store = kj::mv(backing) }); - } +void ReadableStreamDefaultController::error(jsg::Lock& js, v8::Local reason) { + impl.doError(js, js.v8Ref(reason)); +} - pullInto.filled -= remainderSize; - commitPullInto(js, kj::mv(pullInto)); - pullIntoUsingQueue(js); - } +void ReadableStreamDefaultController::pull(jsg::Lock& js) { + // When a consumer receives a read request, but does not have the data available to + // fulfill the request, the consumer will call pull on the controller to pull that + // data if needed. impl.pullIfNeeded(js, JSG_THIS); } -void ReadableByteStreamController::setup( - jsg::Lock& js, - UnderlyingSource underlyingSource, - StreamQueuingStrategy queuingStrategy) { - int autoAllocateChunkSize = - underlyingSource.autoAllocateChunkSize.orDefault( - UnderlyingSource::DEFAULT_AUTO_ALLOCATE_CHUNK_SIZE); - JSG_REQUIRE(autoAllocateChunkSize > 0, - TypeError, - "The autoAllocateChunkSize option cannot be zero."); - this->autoAllocateChunkSize = autoAllocateChunkSize; - - impl.setup(js, JSG_THIS, kj::mv(underlyingSource), kj::mv(queuingStrategy)); -} - -size_t ReadableByteStreamController::updatePullInto(jsg::Lock& js, jsg::BufferSource view) { -// TODO(stream-tee): This likely goes away and is handled by the ByteQueue::Consumer. - auto& pullInto = pendingPullIntos.front(); - auto byteLength = view.size(); - JSG_REQUIRE(view.canDetach(js), TypeError, - "Unable to use non-detachable ArrayBuffer."); - JSG_REQUIRE(pullInto.store.getOffset() + pullInto.filled == view.getOffset(), - RangeError, - "The given view has an invalid byte offset."); - JSG_REQUIRE(pullInto.store.size() == view.underlyingArrayBufferSize(js), - RangeError, - "The underlying ArrayBuffer is not the correct length."); - JSG_REQUIRE(pullInto.filled + byteLength <= pullInto.store.size(), - RangeError, - "The view is not the correct length."); - pullInto.store = view.detach(js); - return byteLength; +kj::Own ReadableStreamDefaultController::getConsumer( + kj::Maybe stateListener) { + return impl.getConsumer(stateListener); } // ====================================================================================== -// TODO(stream-tee): Everything JsTeeController goes away - -ReadableStreamJsTeeController::Attached::Attached( - jsg::Ref ref, - TeeController& controller) - : ref(kj::mv(ref)), controller(controller) {}; - -ReadableStreamJsTeeController::ReadableStreamJsTeeController( - jsg::Ref baseStream, - TeeController& teeController) - : state(Readable()), - innerState(Attached(kj::mv(baseStream), teeController)) {} - -ReadableStreamJsTeeController::ReadableStreamJsTeeController( - jsg::Lock& js, - kj::Maybe attached, - Queue& queue) - : state(Readable()), - innerState(kj::mv(attached)), - queue(copyQueue(queue, js)) {} - -ReadableStreamJsTeeController::ReadableStreamJsTeeController(ReadableStreamJsTeeController&& other) - : owner(kj::mv(other.owner)), - state(kj::mv(other.state)), - innerState(kj::mv(other.innerState)), - lock(kj::mv(other.lock)), - disturbed(other.disturbed) {} - -ReadableStreamJsTeeController::Queue ReadableStreamJsTeeController::copyQueue( - Queue& queue, - jsg::Lock& js) { - ReadableStreamJsTeeController::Queue newQueue; - for (auto& item : queue) { - KJ_IF_MAYBE(value, item.value) { - newQueue.push_back(ReadResult { .value = value->addRef(js), .done = item.done }); - } else { - newQueue.push_back(ReadResult { .done = item.done }); +void ReadableStreamBYOBRequest::visitForGc(jsg::GcVisitor& visitor) { + KJ_IF_MAYBE(impl, maybeImpl) { + visitor.visit(impl->view, impl->controller); } } - return kj::mv(newQueue); -} - -ReadableStreamJsTeeController::~ReadableStreamJsTeeController() noexcept(false) { - // There's a good chance that we're cleaning up during garbage collection here. - // In that case, we don't want detach to go off and cancel any remainin read - // promises as that would potentially involve allocating JS stuff during GC, - // which is a no no. - detach(nullptr); -}; - -jsg::Ref ReadableStreamJsTeeController::addRef() { - return KJ_ASSERT_NONNULL(owner).addRef(); -} -jsg::Promise ReadableStreamJsTeeController::cancel( +ReadableStreamBYOBRequest::ReadableStreamBYOBRequest( jsg::Lock& js, - jsg::Optional> reason) { - disturbed = true; - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) { - return js.resolvedPromise(); - } - KJ_CASE_ONEOF(errored, StreamStates::Errored) { - return js.rejectedPromise(errored.addRef(js)); - } - KJ_CASE_ONEOF(readable, Readable) { - doCancel(js, reason.orDefault(js.v8Undefined())); - return js.resolvedPromise(); - } - } - KJ_UNREACHABLE; -} + kj::Own readRequest, + jsg::Ref controller) + : maybeImpl(Impl(js, kj::mv(readRequest), kj::mv(controller))) {} -void ReadableStreamJsTeeController::detach(kj::Maybe maybeJs) { - KJ_IF_MAYBE(inner, innerState) { - inner->controller.removeBranch(this, maybeJs); +kj::Maybe ReadableStreamBYOBRequest::getAtLeast() { + KJ_IF_MAYBE(impl, maybeImpl) { + return impl->readRequest->getAtLeast(); } - innerState = nullptr; -} - -void ReadableStreamJsTeeController::doCancel(jsg::Lock& js, v8::Local reason) { - // Canceling a tee controller does several things: - // 1. Clears the queue - // 2. Sets both the state and innerState to closed. - // 3. Flushes remaining read requests - queue.clear(); - finishClosing(js); -} - -void ReadableStreamJsTeeController::doClose() { - // doClose is called by the inner TeeController to signal that the inner side is closed. - closePending = true; + return nullptr; } -void ReadableStreamJsTeeController::drain(kj::Maybe> maybeReason) { - KJ_IF_MAYBE(reason, maybeReason) { - while (!readRequests.empty()) { - auto request = kj::mv(readRequests.front()); - readRequests.pop_front(); - request.reject(*reason); - } - return; - } - while (!readRequests.empty()) { - auto request = kj::mv(readRequests.front()); - readRequests.pop_front(); - request.resolve({ .done = true }); +kj::Maybe> ReadableStreamBYOBRequest::getView(jsg::Lock& js) { + KJ_IF_MAYBE(impl, maybeImpl) { + return impl->view.addRef(js); } + return nullptr; } -void ReadableStreamJsTeeController::doError(jsg::Lock& js, v8::Local reason) { - // doError is called by the inner TeeController to signal that the inner side has - // errored. This outer controller must detach itself, clear the queue, and transition - // itself into the errored state as well. - detach(js); - state.init(js.v8Ref(reason)); - queue.clear(); - - drain(reason); - - KJ_SWITCH_ONEOF(lock.state) { - KJ_CASE_ONEOF(locked, ReaderLocked) { - maybeRejectPromise(locked.getClosedFulfiller(), reason); - } - KJ_CASE_ONEOF(locked, ReadableLockImpl::PipeLocked) { - lock.state.init(); - } - KJ_CASE_ONEOF(locked, ReadableLockImpl::TeeLocked) { - // This state is unreachable because the TeeLocked state is not - // used by ReadableStreamJsTeeController. - KJ_UNREACHABLE; - } - KJ_CASE_ONEOF(locked, Unlocked) {} - KJ_CASE_ONEOF(locked, Locked) {} +void ReadableStreamBYOBRequest::invalidate(jsg::Lock& js) { + KJ_IF_MAYBE(impl, maybeImpl) { + // If the user code happened to have retained a reference to the view or + // the buffer, we need to detach it so that those references cannot be used + // to modify or observe modifications. + impl->view.getHandle(js)->Buffer()->Detach(); + impl->controller->maybeByobRequest = nullptr; } + maybeImpl = nullptr; } -void ReadableStreamJsTeeController::finishClosing(jsg::Lock& js) { - detach(js); - state.init(); - - drain(nullptr); +void ReadableStreamBYOBRequest::respond(jsg::Lock& js, int bytesWritten) { + auto& impl = JSG_REQUIRE_NONNULL(maybeImpl, + TypeError, + "This ReadableStreamBYOBRequest has been invalidated."); + bool pull = false; + if (!impl.controller->canCloseOrEnqueue()) { + JSG_REQUIRE(bytesWritten == 0, + TypeError, + "The bytesWritten must be zero after the stream is closed."); + KJ_ASSERT(impl.readRequest->isInvalidated()); + } else { + JSG_REQUIRE(bytesWritten > 0, + TypeError, + "The bytesWritten must be more than zero while the stream is open."); + impl.readRequest->respond(js, bytesWritten); + pull = true; + } - KJ_SWITCH_ONEOF(lock.state) { - KJ_CASE_ONEOF(locked, ReaderLocked) { - maybeResolvePromise(locked.getClosedFulfiller()); - } - KJ_CASE_ONEOF(locked, ReadableLockImpl::PipeLocked) { - lock.state.init(); - } - KJ_CASE_ONEOF(locked, ReadableLockImpl::TeeLocked) { - // This state is unreachable because the TeeLocked state is not - // used by ReadableStreamJsTeeController. - KJ_UNREACHABLE; - } - KJ_CASE_ONEOF(locked, Unlocked) {} - KJ_CASE_ONEOF(locked, Locked) {} + KJ_DEFER(invalidate(js)); + if (pull) { + impl.controller->pull(js); } } -void ReadableStreamJsTeeController::handleData(jsg::Lock& js, ReadResult result) { - // handleData is called by the inner TeeController when data has been ready from the underlying - // source. If there are pending read requests, fulfill the first one immediately, otherwise - // push the item on the queue. - if (!readRequests.empty()) { - KJ_ASSERT(queue.empty()); - auto request = kj::mv(readRequests.front()); - readRequests.pop_front(); - request.resolve(kj::mv(result)); - - // If the innerState has been detached and there are no further read requests, - // transition into the closed state. - if (closePending) { - finishClosing(js); - } - - return; +void ReadableStreamBYOBRequest::respondWithNewView(jsg::Lock& js, jsg::BufferSource view) { + auto& impl = JSG_REQUIRE_NONNULL(maybeImpl, + TypeError, + "This ReadableStreamBYOBRequest has been invalidated."); + bool pull = false; + if (!impl.controller->canCloseOrEnqueue()) { + JSG_REQUIRE(view.size() == 0, + TypeError, + "The view byte length must be zero after the stream is closed."); + } else { + JSG_REQUIRE(view.size() > 0, + TypeError, + "The view byte length must be more than zero while the stream is open."); + impl.readRequest->respondWithNewView(js, kj::mv(view)); + pull = true; } - queue.push_back(kj::mv(result)); -} -bool ReadableStreamJsTeeController::hasPendingReadRequests() { - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) { - KJ_ASSERT(readRequests.empty()); - return false; - } - KJ_CASE_ONEOF(errored, StreamStates::Errored) { - KJ_ASSERT(readRequests.empty()); - return false; - } - KJ_CASE_ONEOF(readable, Readable) { - return !readRequests.empty(); - } + KJ_DEFER(invalidate(js)); + if (pull) { + impl.controller->pull(js); } - KJ_UNREACHABLE; } -bool ReadableStreamJsTeeController::isByteOriented() const { return false; }; +// ====================================================================================== -bool ReadableStreamJsTeeController::isDisturbed() { return disturbed; } +ReadableByteStreamController::ReadableByteStreamController( + UnderlyingSource underlyingSource, + StreamQueuingStrategy queuingStrategy) + : impl(kj::mv(underlyingSource), kj::mv(queuingStrategy)) {} -bool ReadableStreamJsTeeController::isLockedToReader() const { - return lock.isLockedToReader(); +void ReadableByteStreamController::start(jsg::Lock& js) { + impl.start(js, JSG_THIS); } -bool ReadableStreamJsTeeController::isClosedOrErrored() const { - return state.is() || state.is(); +bool ReadableByteStreamController::canCloseOrEnqueue() { + return impl.canCloseOrEnqueue(); } -bool ReadableStreamJsTeeController::lockReader(jsg::Lock& js, Reader& reader) { - return lock.lockReader(js, *this, reader); +bool ReadableByteStreamController::hasBackpressure() { + return !impl.shouldCallPull(); } -jsg::Promise ReadableStreamJsTeeController::pipeTo( - jsg::Lock& js, - WritableStreamController& destination, - PipeToOptions options) { - KJ_DASSERT(!isLockedToReader()); - KJ_DASSERT(!destination.isLockedToWriter()); +kj::Maybe ReadableByteStreamController::getDesiredSize() { + return impl.getDesiredSize(); +} - disturbed = true; - KJ_IF_MAYBE(promise, destination.tryPipeFrom(js, addRef(), kj::mv(options))) { - return kj::mv(*promise); - } +bool ReadableByteStreamController::hasPendingReadRequests() { + return impl.hasPendingReadRequests(); +} - return js.rejectedPromise( - js.v8TypeError("This ReadableStream cannot be piped to this WritableStream."_kj)); +void ReadableByteStreamController::visitForGc(jsg::GcVisitor& visitor) { + visitor.visit(maybeByobRequest, impl); } -kj::Maybe> ReadableStreamJsTeeController::read( +jsg::Promise ReadableByteStreamController::cancel( jsg::Lock& js, - kj::Maybe byobOptions) { - disturbed = true; - // Per the streams specification, ReadableStream tee branches do not support BYOB reads. - // The byobOptions should never be set here, but let's make sure. - KJ_ASSERT(byobOptions == nullptr); - - if (state.is()) { - KJ_ASSERT(queue.empty()); - return js.resolvedPromise(ReadResult { .done = true }); - } - - KJ_IF_MAYBE(errored, state.tryGet()) { - KJ_ASSERT(queue.empty()); - return js.rejectedPromise(errored->addRef(js)); - } - - // Every tee controller has its own internal queue. - // If that internal queue is not empty, read will pull from it, - // otherwise, the read request will be queued and the underlying tee controller - // will be asked to pull data. When the controller does pull data, it will be - // delivered to every branch. If the branch queue is not empty, or there - // are no pending reads, the data will be appended into the tee controller's - // queue. If there are pending reads, the queue should be empty and the - // next pending read will be fulfilled. + jsg::Optional> maybeReason) { + return impl.cancel(js, JSG_THIS, maybeReason.orDefault(js.v8Undefined())); +} - // First, let's check the internal queue. If there's data, we can resolve - // the read promise immediately. - if (!queue.empty()) { - // The tee controller queue will only ever have value items. - auto item = kj::mv(queue.front()); - queue.pop_front(); +void ReadableByteStreamController::close(jsg::Lock& js) { + impl.close(js); +} - // If the innerState has been detached and there are no further read requests, - // transition into the closed state. - if (innerState == nullptr && readRequests.empty()) { - finishClosing(js); - } +void ReadableByteStreamController::enqueue(jsg::Lock& js, jsg::BufferSource chunk) { + JSG_REQUIRE(chunk.size() > 0, TypeError, "Cannot enqueue a zero-length ArrayBuffer."); + JSG_REQUIRE(chunk.canDetach(js), TypeError, + "The provided ArrayBuffer must be detachable."); + JSG_REQUIRE(impl.canCloseOrEnqueue(), TypeError, "This ReadableByteStreamController is closed."); - return js.resolvedPromise(kj::mv(item)); + KJ_IF_MAYBE(byobRequest, maybeByobRequest) { + (*byobRequest)->invalidate(js); } - auto& controller = KJ_ASSERT_NONNULL(innerState).controller; - auto readRequest = js.newPromiseAndResolver(); - readRequests.push_back(kj::mv(readRequest.resolver)); - controller.ensurePulling(js); - return kj::mv(readRequest.promise); + impl.enqueue(js, kj::refcounted(chunk.detach(js)), JSG_THIS); } -void ReadableStreamJsTeeController::releaseReader(Reader& reader, kj::Maybe maybeJs) { - lock.releaseReader(*this, reader, maybeJs); +void ReadableByteStreamController::error(jsg::Lock& js, v8::Local reason) { + impl.doError(js, js.v8Ref(reason)); } -kj::Maybe> -ReadableStreamJsTeeController::removeSource(jsg::Lock& js) { - JSG_REQUIRE(!isLockedToReader(), TypeError, "This ReadableStream is locked to a reader."); - - lock.state.init(); - disturbed = true; - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) { - return kj::refcounted(StreamStates::Closed()); - } - KJ_CASE_ONEOF(errored, StreamStates::Errored) { - kj::throwFatalException(js.exceptionToKj(errored.addRef(js))); - } - KJ_CASE_ONEOF(readable, Readable) { - // It is possible that the tee controller queue already has data in it that data needs to - // be moved over into the ReadableStreamJsTeeSource we are going to create. However, we - // have to make sure that it's all byte data, otherwise we need to error. Also, to make - // reading that data as efficient as possible in the source, we copy it into a queue rather - // than keeping it as individual ReadResult objects. - std::deque bytes; - while (!queue.empty()) { - auto& item = queue.front(); - KJ_IF_MAYBE(value, item.value) { - auto view = value->getHandle(js); - JSG_REQUIRE(view->IsArrayBufferView() || view->IsArrayBuffer(), TypeError, - "This ReadableStream does not contain bytes."); - jsg::BufferSource source(js, view); - auto ptr = source.asArrayPtr(); - std::copy(ptr.begin(), ptr.end(), std::back_inserter(bytes)); - queue.pop_front(); - continue; - } - if (item.done) { - break; - } - } +kj::Maybe> +ReadableByteStreamController::getByobRequest(jsg::Lock& js) { + if (maybeByobRequest == nullptr) { + auto& queue = JSG_REQUIRE_NONNULL( + impl.state.tryGet(), + TypeError, + "This ReadableByteStreamController has been closed."); - KJ_DEFER(state.init()); - auto& inner = KJ_ASSERT_NONNULL(innerState); - auto& controller = inner.controller; - auto ref = inner.ref.addRef(); - detach(js); - return kj::refcounted(kj::mv(ref), controller, kj::mv(bytes)); + KJ_IF_MAYBE(pendingByob, queue.nextPendingByobReadRequest()) { + maybeByobRequest = jsg::alloc(js, + kj::mv(*pendingByob), JSG_THIS); } } - KJ_UNREACHABLE; -} - -void ReadableStreamJsTeeController::setOwnerRef(ReadableStream& owner) { - this->owner = owner; - KJ_ASSERT_NONNULL(innerState).controller.addBranch(this); -} -void ReadableStreamJsTeeController::visitForGc(jsg::GcVisitor& visitor) { - visitor.visit(lock); - KJ_IF_MAYBE(error, state.tryGet()) { - visitor.visit(*error); - } - for (auto& item : queue) { - visitor.visit(item); - } - visitor.visitAll(readRequests); + return maybeByobRequest.map([&](jsg::Ref& req) { + return req.addRef(); + }); } -ReadableStreamController::Tee ReadableStreamJsTeeController::tee(jsg::Lock& js) { - JSG_REQUIRE(!isLockedToReader(), TypeError, - "This ReadableStream is currently locked to a reader."); - disturbed = true; - lock.state.init(); - - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) { - return Tee { - .branch1 = jsg::alloc(ReadableStreamJsController(closed)), - .branch2 = jsg::alloc(ReadableStreamJsController(closed)), - }; - } - KJ_CASE_ONEOF(errored, StreamStates::Errored) { - return Tee { - .branch1 = jsg::alloc(ReadableStreamJsController(errored.addRef(js))), - .branch2 = jsg::alloc(ReadableStreamJsController(errored.addRef(js))), - }; - } - KJ_CASE_ONEOF(readable, Readable) { - if (closePending && queue.empty()) { - finishClosing(js); - return Tee { - .branch1 = jsg::alloc( - ReadableStreamJsController(StreamStates::Closed())), - .branch2 = jsg::alloc( - ReadableStreamJsController(StreamStates::Closed())), - }; - } - - return Tee { - .branch1 = jsg::alloc( - ReadableStreamJsTeeController(js, - innerState.map([](Attached& attached) -> Attached { - return Attached(attached.ref->addRef(), attached.controller); - }), - queue)), - .branch2 = jsg::alloc( - ReadableStreamJsTeeController(js, - innerState.map([](Attached& attached) -> Attached { - return Attached(attached.ref->addRef(), attached.controller); - }), - queue)), - }; - } - } - KJ_UNREACHABLE; +void ReadableByteStreamController::pull(jsg::Lock& js) { + // When a consumer receives a read request, but does not have the data available to + // fulfill the request, the consumer will call pull on the controller to pull that + // data if needed. + impl.pullIfNeeded(js, JSG_THIS); } -kj::Maybe ReadableStreamJsTeeController::tryPipeLock( - jsg::Ref destination) { - return lock.tryPipeLock(*this, kj::mv(destination)); +kj::Own ReadableByteStreamController::getConsumer( + kj::Maybe stateListener) { + return impl.getConsumer(stateListener); } // ====================================================================================== -ReadableStreamJsController::ReadableStreamJsController() {} - ReadableStreamJsController::ReadableStreamJsController(StreamStates::Closed closed) : state(closed) {} ReadableStreamJsController::ReadableStreamJsController(StreamStates::Errored errored) : state(kj::mv(errored)) {} -jsg::Ref ReadableStreamJsController::addRef() { - return KJ_REQUIRE_NONNULL(owner).addRef(); -} - -jsg::Promise ReadableStreamJsController::cancel( +ReadableStreamJsController::ReadableStreamJsController( jsg::Lock& js, - jsg::Optional> maybeReason) { - disturbed = true; - - auto reason = js.v8Ref(maybeReason.orDefault(js.v8Undefined())); - - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) { - return js.resolvedPromise(); - } - KJ_CASE_ONEOF(errored, StreamStates::Errored) { - return js.rejectedPromise(errored.addRef(js)); - } - KJ_CASE_ONEOF(controller, ByobController) { - -// TODO(stream-tee): -// KJ_CASE_ONEOF(byteReadable, kj::Own) { -// return byteReadable->cancel(js, kj::mv(maybeReason)); - - return controller->cancel(js, reason.getHandle(js)); - } - KJ_CASE_ONEOF(controller, DefaultController) { - -// TODO(stream-tee): -// KJ_CASE_ONEOF(valueReadable, kj::Own) { -// return valueReadable->cancel(js, kj::mv(maybeReason)); - - return controller->cancel(js, reason.getHandle(js)); - } - } - - KJ_UNREACHABLE; -} - -void ReadableStreamJsController::doCancel(jsg::Lock& js, v8::Local reason) { -// TODO(stream-tee): Likely not necessary to implement this with the new model? -// This is intended to allow completion of canceling the controller which will be -// done via the ByteReadable or ValueReadable when those are canceled. - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) { - return; - } - KJ_CASE_ONEOF(errored, StreamStates::Errored) { - return; - } - KJ_CASE_ONEOF(controller, ByobController) { - return controller->doCancel(js, reason); - } - KJ_CASE_ONEOF(controller, DefaultController) { - return controller->doCancel(js, reason); - } - } - KJ_UNREACHABLE; -} + ValueReadable& consumer) + : state(consumer.clone(js, this)) {} -void ReadableStreamJsController::detachFromController() { -// TODO(stream-tee): With the new model, the ValueReadable or ByteReadable struct -// will become the new owner of the relationship with the underlying controller. -// There will no longer be a single owner known to the controller, only multiple -// consumers. This function likely goes away entirely in favor of some mechanism -// on the ValueReadable/ByteReadable. - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) {} - KJ_CASE_ONEOF(errored, StreamStates::Errored) {} - KJ_CASE_ONEOF(controller, DefaultController) { - controller->setOwner(nullptr); - } - KJ_CASE_ONEOF(controller, ByobController) { - controller->setOwner(nullptr); - } - } -} - -void ReadableStreamJsController::doClose() { -// TODO(stream-tee): doClose() finalizes the closed state of this ReadableStream. -// The connection to the underlying controller is released with no further action. -// Importantly, this method is triggered by the underlying controller as a result -// of that controller closing or being canceled. We detach ourselves from the -// underlying controller by releasing the ValueReadable or ByteReadable in the -// state and changing that to closed. We also clean up other state here. -// Since the underlying controller will no longer have a single owner, we will -// need to modify things such this signal is triggered by the ValueReadable or -// ByteReadable. - detachFromController(); - state.init(); +ReadableStreamJsController::ReadableStreamJsController( + jsg::Lock& js, + ByteReadable& consumer) + : state(consumer.clone(js, this)) {} - KJ_SWITCH_ONEOF(lock.state) { - KJ_CASE_ONEOF(locked, ReaderLocked) { - maybeResolvePromise(locked.getClosedFulfiller()); - } - KJ_CASE_ONEOF(locked, ReadableLockImpl::PipeLocked) { - lock.state.init(); - } - KJ_CASE_ONEOF(locked, ReadableLockImpl::TeeLocked) { - locked.close(); - } - KJ_CASE_ONEOF(locked, Locked) {} - KJ_CASE_ONEOF(locked, Unlocked) {} - } -} +ReadableStreamJsController::ReadableStreamJsController(kj::Own consumer) + : state(kj::mv(consumer)) {} -void ReadableStreamJsController::controllerClose(jsg::Lock& js) { -// TODO(stream-tee): This is obsolete and no longer needed. - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) { return; } - KJ_CASE_ONEOF(errored, StreamStates::Errored) { return; } - KJ_CASE_ONEOF(controller, DefaultController) { - return controller->close(js); - } - KJ_CASE_ONEOF(controller, ByobController) { - return controller->close(js); - } - } - KJ_UNREACHABLE; +ReadableStreamJsController::ReadableStreamJsController(kj::Own consumer) + : state(kj::mv(consumer)) {} + +jsg::Ref ReadableStreamJsController::addRef() { + return KJ_REQUIRE_NONNULL(owner).addRef(); } -void ReadableStreamJsController::controllerError( +jsg::Promise ReadableStreamJsController::cancel( jsg::Lock& js, - v8::Local reason) { -// TODO(stream-tee): This is obsolete and no longer needed. + jsg::Optional> maybeReason) { + disturbed = true; + + const auto doCancel = [&](auto& consumer) { + auto reason = js.v8Ref(maybeReason.orDefault([&] { return js.v8Undefined(); })); + KJ_DEFER(state.init()); + return consumer->cancel(js, reason.getHandle(js)); + }; + KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) { return; } - KJ_CASE_ONEOF(errored, StreamStates::Errored) { return; } - KJ_CASE_ONEOF(controller, DefaultController) { - return controller->error(js, reason); + KJ_CASE_ONEOF(closed, StreamStates::Closed) { + return js.resolvedPromise(); + } + KJ_CASE_ONEOF(errored, StreamStates::Errored) { + return js.rejectedPromise(errored.addRef(js)); } - KJ_CASE_ONEOF(controller, ByobController) { - return controller->error(js, reason); + KJ_CASE_ONEOF(consumer, kj::Own) { + return doCancel(consumer); + } + KJ_CASE_ONEOF(consumer, kj::Own) { + return doCancel(consumer); } } + KJ_UNREACHABLE; } +void ReadableStreamJsController::doClose() { + // Finalizes the closed state of this ReadableStream. The connection to the underlying + // controller is released with no further action. Importantly, this method is triggered + // by the underlying controller as a result of that controller closing or being canceled. + // We detach ourselves from the underlying controller by releasing the ValueReadable or + // ByteReadable in the state and changing that to closed. + // We also clean up other state here. + state.init(); + lock.onClose(); +} + void ReadableStreamJsController::doError(jsg::Lock& js, v8::Local reason) { -// TODO(stream-tee): As with doClose(), doError() finalizes the error state of this -// ReadableStream. The connection to the underlying controller is released with no -// further action. This method is triggered by the underlying controller as a result -// of that controller erroring or being canceled. We detach ourselves from the -// underlying controller by releasing the ValueReadable or ByteReadable in the state -// and changing that to errored. We also clean up other state here. -// Since the underlying controller will no longer have a single owner, we will -// need to modify things such that this signal is triggered by the ValueReadable or -// ByteReadable. - detachFromController(); + // As with doClose(), doError() finalizes the error state of this ReadableStream. + // The connection to the underlying controller is released with no further action. + // This method is triggered by the underlying controller as a result of that controller + // erroring. We detach ourselves from the underlying controller by releasing the ValueReadable + // or ByteReadable in the state and changing that to errored. + // We also clean up other state here. state.init(js.v8Ref(reason)); - - KJ_SWITCH_ONEOF(lock.state) { - KJ_CASE_ONEOF(locked, ReaderLocked) { - maybeRejectPromise(locked.getClosedFulfiller(), reason); - } - KJ_CASE_ONEOF(locked, ReadableLockImpl::PipeLocked) { - lock.state.init(); - } - KJ_CASE_ONEOF(locked, ReadableLockImpl::TeeLocked) { - locked.error(js, reason); - } - KJ_CASE_ONEOF(locked, Locked) {} - KJ_CASE_ONEOF(locked, Unlocked) {} - } + lock.onError(js, reason); } bool ReadableStreamJsController::hasPendingReadRequests() { KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return false; } KJ_CASE_ONEOF(errored, StreamStates::Errored) { return false; } - KJ_CASE_ONEOF(controller, DefaultController) { - -// TODO(stream-tee): -// KJ_CASE_ONEOF(valueReadable, kj::Own) { -// return valueReadable->consumer->hasPendingReadRequests(); - - return controller->hasPendingReadRequests(); + KJ_CASE_ONEOF(consumer, kj::Own) { + return consumer->hasPendingReadRequests(); } - KJ_CASE_ONEOF(controller, ByobController) { - -// TODO(stream-tee): -// KJ_CASE_ONEOF(byteReadable, kj::Own) { -// return byteReadable->consumer->hasPendingReadRequests(); - - return controller->hasPendingReadRequests(); + KJ_CASE_ONEOF(consumer, kj::Own) { + return consumer->hasPendingReadRequests(); } } KJ_UNREACHABLE; } bool ReadableStreamJsController::isByteOriented() const { -// TODO(stream-tee): -// return state.is>(); - return state.is(); + return state.is>(); } bool ReadableStreamJsController::isClosedOrErrored() const { @@ -2393,15 +1744,6 @@ bool ReadableStreamJsController::isClosedOrErrored() const { bool ReadableStreamJsController::isDisturbed() { return disturbed; } -bool ReadableStreamJsController::isLocked() const { return isLockedToReader(); } - -bool ReadableStreamJsController::isLockedReaderByteOriented() { - KJ_IF_MAYBE(locked, lock.state.tryGet()) { - return locked->getReader().isByteOriented(); - } - return false; -} - bool ReadableStreamJsController::isLockedToReader() const { return lock.isLockedToReader(); } @@ -2461,23 +1803,20 @@ kj::Maybe> ReadableStreamJsController::read( KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { // The closed state for BYOB reads is handled in the maybeByobOptions check above. + KJ_ASSERT(maybeByobOptions == nullptr); return js.resolvedPromise(ReadResult { .done = true }); } KJ_CASE_ONEOF(errored, StreamStates::Errored) { return js.rejectedPromise(errored.addRef(js)); } - KJ_CASE_ONEOF(controller, DefaultController) { - -// TODO(stream-tee): Read for both ValueReadable and ByteReadable must be updated -// in terms of, e.g. valueReadable->consumer->read(...) - + KJ_CASE_ONEOF(consumer, kj::Own) { // The ReadableStreamDefaultController does not support ByobOptions. // It should never happen, but let's make sure. KJ_ASSERT(maybeByobOptions == nullptr); - return controller->read(js); + return consumer->read(js); } - KJ_CASE_ONEOF(controller, ByobController) { - return controller->read(js, kj::mv(maybeByobOptions)); + KJ_CASE_ONEOF(consumer, kj::Own) { + return consumer->read(js, kj::mv(maybeByobOptions)); } } KJ_UNREACHABLE; @@ -2492,9 +1831,9 @@ void ReadableStreamJsController::releaseReader( kj::Maybe> ReadableStreamJsController::removeSource(jsg::Lock& js) { JSG_REQUIRE(!isLockedToReader(), TypeError, "This ReadableStream is locked to a reader."); - lock.state.init(); disturbed = true; + KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return kj::refcounted(StreamStates::Closed()); @@ -2502,59 +1841,63 @@ ReadableStreamJsController::removeSource(jsg::Lock& js) { KJ_CASE_ONEOF(errored, StreamStates::Errored) { kj::throwFatalException(js.exceptionToKj(errored.addRef(js))); } - KJ_CASE_ONEOF(controller, ByobController) { - -// TODO(stream-tee): Removing the source here becomes as simple as transferring the -// ValueReadable and ByteReadable from the ReadableStreamJsController to the -// ReadableStreamJsSource that we're creating. All of the relevant state transfers -// with it. We must, however, ensure that there is still a way for that to communicate -// state changes (e.g. closed, errored) with whichever thing is currently owning it. - + KJ_CASE_ONEOF(consumer, kj::Own) { KJ_DEFER(state.init()); - return kj::refcounted(kj::mv(controller)); + return kj::refcounted(kj::mv(consumer)); } - KJ_CASE_ONEOF(controller, DefaultController) { + KJ_CASE_ONEOF(consumer, kj::Own) { KJ_DEFER(state.init()); - return kj::refcounted(kj::mv(controller)); + return kj::refcounted(kj::mv(consumer)); } } KJ_UNREACHABLE; } ReadableStreamController::Tee ReadableStreamJsController::tee(jsg::Lock& js) { -// TODO(stream-tee): Here, rather than creating new ReadableStreamJsTeeController-backed things, -// we are going to just create new ReadableStreamJsController things that have their own -// clones of this streams ValueReadable or ByteReadable. - KJ_IF_MAYBE(teeController, lock.tryTeeLock(*this)) { - disturbed = true; + JSG_REQUIRE(!isLockedToReader(), TypeError, "This ReadableStream is locked to a reader."); + lock.state.init(); + disturbed = true; - if (state.is()) { + KJ_SWITCH_ONEOF(state) { + KJ_CASE_ONEOF(closed, StreamStates::Closed) { + return Tee { + .branch1 = jsg::alloc( + kj::heap(StreamStates::Closed())), + .branch2 = jsg::alloc( + kj::heap(StreamStates::Closed())), + }; + } + KJ_CASE_ONEOF(errored, StreamStates::Errored) { return Tee { - .branch1 = jsg::alloc(ReadableStreamJsController(StreamStates::Closed())), - .branch2 = jsg::alloc(ReadableStreamJsController(StreamStates::Closed())), + .branch1 = jsg::alloc(kj::heap( + errored.addRef(js))), + .branch2 = jsg::alloc(kj::heap( + errored.addRef(js))), }; } - - KJ_IF_MAYBE(errored, state.tryGet()) { + KJ_CASE_ONEOF(consumer, kj::Own) { + KJ_DEFER(state.init()); return Tee { - .branch1 = jsg::alloc(ReadableStreamJsController(errored->addRef(js))), - .branch2 = jsg::alloc(ReadableStreamJsController(errored->addRef(js))), + .branch1 = jsg::alloc(kj::heap(js, *consumer)), + .branch2 = jsg::alloc(kj::heap( + kj::mv(consumer))), + }; + } + KJ_CASE_ONEOF(consumer, kj::Own) { + KJ_DEFER(state.init()); + return Tee { + .branch1 = jsg::alloc(kj::heap(js, *consumer)), + .branch2 = jsg::alloc(kj::heap( + kj::mv(consumer))), }; } - - return Tee { - .branch1 = jsg::alloc( - ReadableStreamJsTeeController(addRef(), *teeController)), - .branch2 = jsg::alloc( - ReadableStreamJsTeeController(addRef(), *teeController)), - }; } - JSG_FAIL_REQUIRE(TypeError, "This ReadableStream is currently locked to a reader."); + KJ_UNREACHABLE; } void ReadableStreamJsController::setOwnerRef(ReadableStream& stream) { KJ_ASSERT(owner == nullptr); - owner = stream; + owner = &stream; } void ReadableStreamJsController::setup( @@ -2567,24 +1910,28 @@ void ReadableStreamJsController::setup( maybeTransformer = kj::mv(underlyingSource.maybeTransformer); -// TODO(stream-tee): Here, we wrap create the underlying controllers but we need to -// create the ValueReadable or ByteReadable and use that for the state instead. - if (type == "bytes") { - state = jsg::alloc(*this); - state.get()->setup( - js, + auto autoAllocateChunkSize = underlyingSource.autoAllocateChunkSize.orDefault( + UnderlyingSource::DEFAULT_AUTO_ALLOCATE_CHUNK_SIZE); + + auto controller = jsg::alloc( kj::mv(underlyingSource), kj::mv(queuingStrategy)); + + JSG_REQUIRE(autoAllocateChunkSize > 0, + TypeError, + "The autoAllocateChunkSize option cannot be zero."); + + state = kj::refcounted(controller.addRef(), this, autoAllocateChunkSize); + controller->start(js); } else { - JSG_REQUIRE(type == "", - TypeError, - kj::str("\"", type, "\" is not a valid type of ReadableStream.")); - state = jsg::alloc(*this); - state.get()->setup( - js, + JSG_REQUIRE(type == "", TypeError, + kj::str("\"", type, "\" is not a valid type of ReadableStream.")); + auto controller = jsg::alloc( kj::mv(underlyingSource), kj::mv(queuingStrategy)); + state = kj::refcounted(controller.addRef(), this); + controller->start(js); } } @@ -2599,39 +1946,25 @@ void ReadableStreamJsController::visitForGc(jsg::GcVisitor& visitor) { KJ_CASE_ONEOF(error, StreamStates::Errored) { visitor.visit(error); } - KJ_CASE_ONEOF(controller, DefaultController) { - -// TODO(stream-tee): -// KJ_CASE_ONEOF(valueReadable, kj::Own) { -// visitor.visit(*valueReadable); - - visitor.visit(controller); + KJ_CASE_ONEOF(consumer, kj::Own) { + visitor.visit(*consumer); } - KJ_CASE_ONEOF(controller, ByobController) { - -// TODO(stream-tee): -// KJ_CASE_ONEOF(byteReadable, kj::Own) { -// visitor.visit(*byteReadable); - - visitor.visit(controller); + KJ_CASE_ONEOF(consumer, kj::Own) { + visitor.visit(*consumer); } } visitor.visit(lock, maybeTransformer); }; kj::Maybe ReadableStreamJsController::getDesiredSize() { -// TODO(stream-tee): This is used by the TransformStream implementation. This needs to be -// implemented in terms of the underlying controllers backpressure, such that even if there -// is no data waiting in this particular stream's buffer, backpressure is still being signalled -// correctly through every consumer branch. KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return nullptr; } KJ_CASE_ONEOF(errored, StreamStates::Errored) { return nullptr; } - KJ_CASE_ONEOF(controller, DefaultController) { - return controller->getDesiredSize(); + KJ_CASE_ONEOF(consumer, kj::Own) { + return consumer->getDesiredSize(); } - KJ_CASE_ONEOF(controller, ByobController) { - return controller->getDesiredSize(); + KJ_CASE_ONEOF(consumer, kj::Own) { + return consumer->getDesiredSize(); } } KJ_UNREACHABLE; @@ -2644,116 +1977,99 @@ kj::Maybe> ReadableStreamJsController::isErrored(jsg::Lock& } bool ReadableStreamJsController::canCloseOrEnqueue() { -// TODO(stream-tee): This is used by the TransformStream implementation. The implementation -// here won't need to change much. KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return false; } KJ_CASE_ONEOF(errored, StreamStates::Errored) { return false; } - KJ_CASE_ONEOF(controller, DefaultController) { - return controller->canCloseOrEnqueue(); + KJ_CASE_ONEOF(consumer, kj::Own) { + return consumer->canCloseOrEnqueue(); } - KJ_CASE_ONEOF(controller, ByobController) { - return controller->canCloseOrEnqueue(); + KJ_CASE_ONEOF(consumer, kj::Own) { + return consumer->canCloseOrEnqueue(); } } KJ_UNREACHABLE; } bool ReadableStreamJsController::hasBackpressure() { -// TODO(stream-tee): This is used by the TransformStream implementation. Need to determine -// however if we can get rid of this and just use negative or zero desiredSize. + KJ_IF_MAYBE(size, getDesiredSize()) { return *size <= 0; } + return false; +} + +kj::Maybe, + jsg::Ref>> +ReadableStreamJsController::getController() { KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) { return false; } - KJ_CASE_ONEOF(errored, StreamStates::Errored) { return false; } - KJ_CASE_ONEOF(controller, DefaultController) { - return controller->hasBackpressure(); + KJ_CASE_ONEOF(closed, StreamStates::Closed) { return nullptr; } + KJ_CASE_ONEOF(errored, StreamStates::Errored) { return nullptr; } + KJ_CASE_ONEOF(consumer, kj::Own) { + return consumer->getControllerRef(); } - KJ_CASE_ONEOF(controller, ByobController) { - return controller->hasBackpressure(); + KJ_CASE_ONEOF(consumer, kj::Own) { + return consumer->getControllerRef(); } } KJ_UNREACHABLE; } -void ReadableStreamJsController::defaultControllerEnqueue( - jsg::Lock& js, - v8::Local chunk) { -// TODO(stream-tee): Is this necessary still? - auto& controller = KJ_ASSERT_NONNULL(state.tryGet(), - "defaultControllerEnqueue() can only be called with a ReadableStreamDefaultController"); - controller->doEnqueue(js, chunk); +// ====================================================================================== + +ReadableStreamJsSource::ReadableStreamJsSource(StreamStates::Closed closed) + : ioContext(IoContext::current()), + state(closed), + readPending(false) {} + +ReadableStreamJsSource::ReadableStreamJsSource(kj::Exception errored) + : ioContext(IoContext::current()), + state(kj::mv(errored)), + readPending(false) {} + +ReadableStreamJsSource::ReadableStreamJsSource(kj::Own consumer) + : ioContext(IoContext::current()), + state(kj::mv(consumer)) { + state.get>()->setOwner(this); } -// ====================================================================================== +ReadableStreamJsSource::ReadableStreamJsSource(kj::Own consumer) + : ioContext(IoContext::current()), + state(kj::mv(consumer)) { + state.get>()->setOwner(this); +} void ReadableStreamJsSource::cancel(kj::Exception reason) { - const auto doCancel = [this](auto& controller, auto reason) { - JSG_REQUIRE(!canceling, TypeError, "The stream has already been canceled."); - canceling = true; - -// TODO(stream-tee): The change here will echo what is happening over in JsController. -// Specifically, the JsSource will have either a ValueReadable or ByteReadable that -// is one of several owning a reference to the underlying controller. When the last -// one canceled is the one that actually triggers the underlying controller cancel. - - ioContext.addTask(ioContext.run([this, &controller, reason = kj::mv(reason)] - (Worker::Lock& lock) mutable -> kj::Promise { - detachFromController(); + const auto doCancel = [this](auto& consumer, auto reason) { + auto c = kj::mv(consumer); + state.init(kj::cp(reason)); + ioContext.addTask(ioContext.run( + [consumer = kj::mv(c), reason = kj::mv(reason)] + (Worker::Lock& lock) mutable -> kj::Promise { jsg::Lock& js = lock; - state.init(kj::cp(reason)); v8::HandleScope handleScope(js.v8Isolate); - return ioContext.awaitJs( - controller->cancel(js, js.exceptionToJs(kj::cp(reason)).getHandle(js)).then(js, - [this](jsg::Lock& js) { canceling = false; }, - [this](jsg::Lock& js, jsg::Value reason) { - canceling = false; - js.throwException(kj::mv(reason)); - })); - }).attach(controller.addRef(), kj::addRef(*this))); + return IoContext::current().awaitJs( + consumer->cancel(js, js.exceptionToJs(kj::cp(reason)).getHandle(js))); + }).attach(kj::addRef(*this))); }; KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { return; } KJ_CASE_ONEOF(errored, kj::Exception) { kj::throwFatalException(kj::cp(errored)); } - KJ_CASE_ONEOF(controller, ByobController) { doCancel(controller, kj::mv(reason)); } - KJ_CASE_ONEOF(controller, DefaultController) { doCancel(controller, kj::mv(reason)); } - } -} - -void ReadableStreamJsSource::detachFromController() { -// TODO(stream-tee): Since it will be the ValueReadable or ByteReadable that deals -// with this now, hopefully we can remove this. - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) {} - KJ_CASE_ONEOF(errored, kj::Exception) {} - KJ_CASE_ONEOF(controller, DefaultController) { - controller->setOwner(nullptr); + KJ_CASE_ONEOF(consumer, kj::Own) { + return doCancel(consumer, kj::mv(reason)); } - KJ_CASE_ONEOF(controller, ByobController) { - controller->setOwner(nullptr); + KJ_CASE_ONEOF(consumer, kj::Own) { + return doCancel(consumer, kj::mv(reason)); } } + KJ_UNREACHABLE; } void ReadableStreamJsSource::doClose() { -// TODO(stream-tee): Similar change here as JsController::doClose - detachFromController(); state.init(); } void ReadableStreamJsSource::doError(jsg::Lock& js, v8::Local reason) { -// TODO(stream-tee): Similar change here as JsController::doError - detachFromController(); state.init(js.exceptionToKj(js.v8Ref(reason))); } -bool ReadableStreamJsSource::isLocked() const { return true; } - -bool ReadableStreamJsSource::isLockedReaderByteOriented() { return true; } - -// TODO(stream-tee): The read mechanisms below must be updated in terms of using -// the ValueReadable or ByteReadable. - jsg::Promise ReadableStreamJsSource::readFromByobController( jsg::Lock& js, void* buffer, @@ -2780,9 +2096,9 @@ jsg::Promise ReadableStreamJsSource::readFromByobController( .atLeast = minBytes, }; - auto& controller = KJ_ASSERT_NONNULL(state.tryGet()); + auto& consumer = state.get>(); - return controller->read(js, kj::mv(byobOptions)) + return consumer->read(js, kj::mv(byobOptions)) .then(js, [this, buffer, maxBytes, minBytes] (jsg::Lock& js, ReadResult result) mutable -> jsg::Promise { size_t byteLength = 0; @@ -2790,11 +2106,12 @@ jsg::Promise ReadableStreamJsSource::readFromByobController( jsg::BufferSource source(js, value->getHandle(js)); KJ_ASSERT(source.size() <= maxBytes); byteLength = source.size(); - memcpy(reinterpret_cast(buffer), source.asArrayPtr().begin(), byteLength); + auto ptr = source.asArrayPtr().begin(); + std::copy(ptr, ptr + byteLength, reinterpret_cast(buffer)); } if (result.done) { doClose(); - } else if (byteLength < minBytes) { + } else if (byteLength < minBytes && state.is>()) { // If byteLength is less than minBytes and we're not done, we should do another read in // order to fulfill the minBytes contract. When doing so, we adjust the buffer pointer up // by byteLength and reduce both the minBytes and maxBytes by byteLength. Ideally this @@ -2810,9 +2127,8 @@ jsg::Promise ReadableStreamJsSource::readFromByobController( } return js.resolvedPromise(kj::cp(byteLength)); }, [this](jsg::Lock& js, jsg::Value reason) -> jsg::Promise { - detachFromController(); - state.init(js.exceptionToKj(reason.addRef(js))); - js.throwException(kj::mv(reason)); + doError(js, reason.getHandle(js)); + return js.rejectedPromise(kj::mv(reason)); }); } @@ -2832,9 +2148,7 @@ jsg::Promise ReadableStreamJsSource::readFromDefaultController( // Good news, we can fulfill the minimum requirements of this tryRead // synchronously from the queue. auto bytesToCopy = kj::min(maxBytes, queue.size()); - std::copy(queue.begin(), - queue.begin() + bytesToCopy, - ptr.begin()); + std::copy(queue.begin(), queue.begin() + bytesToCopy, ptr.begin()); queue.erase(queue.begin(), queue.begin() + bytesToCopy); return js.resolvedPromise(kj::cp(bytesToCopy)); } @@ -2845,9 +2159,7 @@ jsg::Promise ReadableStreamJsSource::readFromDefaultController( if (bytesToCopy > 0) { // This should be true because if it wasn't we would have caught it above. KJ_ASSERT(bytesToCopy < minBytes); - std::copy(queue.begin(), - queue.begin() + bytesToCopy, - ptr.begin()); + std::copy(queue.begin(), queue.begin() + bytesToCopy, ptr.begin()); queue.clear(); bytes += bytesToCopy; minBytes -= bytesToCopy; @@ -2875,9 +2187,9 @@ jsg::Promise ReadableStreamJsSource::readLoop( KJ_CASE_ONEOF(errored, kj::Exception) { return js.rejectedPromise(js.exceptionToJs(kj::cp(errored))); } - KJ_CASE_ONEOF(controller, ByobController) { KJ_UNREACHABLE; } - KJ_CASE_ONEOF(controller, DefaultController) { - return controller->read(js).then(js, + KJ_CASE_ONEOF(consumer, kj::Own) { KJ_UNREACHABLE; } + KJ_CASE_ONEOF(consumer, kj::Own) { + return consumer->read(js).then(js, [this, bytes, minBytes, maxBytes, amount] (jsg::Lock& js, ReadResult result) mutable -> jsg::Promise { @@ -2904,14 +2216,14 @@ jsg::Promise ReadableStreamJsSource::readLoop( // increment amount by maxBytes, push the remaining bytes onto the queue, and // return amount. if (bufferSource.size() > maxBytes) { - memcpy(bytes, ptr.begin(), maxBytes); + std::copy(ptr.begin(), ptr.begin() + maxBytes, bytes); std::copy(ptr.begin() + maxBytes, ptr.end(), std::back_inserter(queue)); amount += maxBytes; return js.resolvedPromise(kj::cp(amount)); } KJ_ASSERT(bufferSource.size() <= maxBytes); - memcpy(bytes, ptr.begin(), bufferSource.size()); + std::copy(ptr.begin(), ptr.begin() + bufferSource.size(), bytes); amount += bufferSource.size(); // We've met the minimum requirements! Go ahead and return. The worst case @@ -2928,9 +2240,8 @@ jsg::Promise ReadableStreamJsSource::readLoop( return readLoop(js, bytes, minBytes, maxBytes, amount); }, [this](jsg::Lock& js, jsg::Value reason) -> jsg::Promise { - detachFromController(); - state.init(js.exceptionToKj(reason.addRef(js))); - js.throwException(kj::mv(reason)); + doError(js, reason.getHandle(js)); + return js.rejectedPromise(kj::mv(reason)); }); } } @@ -2943,25 +2254,8 @@ kj::Promise ReadableStreamJsSource::tryRead( size_t maxBytes) { return ioContext.run([this, buffer, minBytes, maxBytes](Worker::Lock& lock) -> kj::Promise { - jsg::Lock& js = lock; - // Of particular note here: Notice that we attach a reference to this and the controller - // if it exists. This is to ensure that both the kj and js heap objects are live until - // the promise resolves. - auto promise = ioContext.awaitJs(internalTryRead(js, buffer, minBytes, maxBytes)) + return ioContext.awaitJs(internalTryRead(lock, buffer, minBytes, maxBytes)) .attach(kj::addRef((*this))); - - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) {} - KJ_CASE_ONEOF(errored, kj::Exception) {} - KJ_CASE_ONEOF(controller, DefaultController) { - promise = promise.attach(controller.addRef()); - } - KJ_CASE_ONEOF(controller, ByobController) { - promise = promise.attach(controller.addRef()); - } - } - - return kj::mv(promise); }); } @@ -2976,9 +2270,7 @@ jsg::Promise ReadableStreamJsSource::internalTryRead( // There's still data in the queue. Copy it out until the queue is empty. auto bytesToCopy = kj::min(maxBytes, queue.size()); auto ptr = kj::ArrayPtr(static_cast(buffer), bytesToCopy); - std::copy(queue.begin(), - queue.begin() + bytesToCopy, - ptr.begin()); + std::copy(queue.begin(), queue.begin() + bytesToCopy, ptr.begin()); queue.erase(queue.begin(), queue.begin() + bytesToCopy); return js.resolvedPromise(kj::cp(bytesToCopy)); } @@ -2987,7 +2279,7 @@ jsg::Promise ReadableStreamJsSource::internalTryRead( KJ_CASE_ONEOF(errored, kj::Exception) { return js.rejectedPromise(js.exceptionToJs(kj::cp(errored))); } - KJ_CASE_ONEOF(controller, DefaultController) { + KJ_CASE_ONEOF(consumer, kj::Own) { JSG_REQUIRE(!readPending, TypeError, "There is already a read pending."); readPending = true; @@ -3000,7 +2292,7 @@ jsg::Promise ReadableStreamJsSource::internalTryRead( js.throwException(kj::mv(reason)); }); } - KJ_CASE_ONEOF(controller, ByobController) { + KJ_CASE_ONEOF(consumer, kj::Own) { JSG_REQUIRE(!readPending, TypeError, "There is already a read pending."); readPending = true; @@ -3026,21 +2318,8 @@ kj::Promise> ReadableStreamJsSource::pumpTo( // Of particular note here: Notice that we attach a reference to this and the controller // if it exists. This is to ensure that both the kj and js heap objects are live until // the promise resolves. - auto promise = ioContext.awaitJs(pipeLoop(js, output, end, kj::heapArray(4096))) + return ioContext.awaitJs(pipeLoop(js, output, end, kj::heapArray(4096))) .attach(kj::addRef(*this)); - - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) {} - KJ_CASE_ONEOF(errored, kj::Exception) {} - KJ_CASE_ONEOF(controller, ByobController) { - promise = promise.attach(controller.addRef()); - } - KJ_CASE_ONEOF(controller, DefaultController) { - promise = promise.attach(controller.addRef()); - } - } - - return kj::mv(promise); })); } @@ -3049,299 +2328,40 @@ jsg::Promise ReadableStreamJsSource::pipeLoop( WritableStreamSink& output, bool end, kj::Array bytes) { - return internalTryRead(js, bytes.begin(), 1, bytes.size()) - .then(js, [this, &output, end, bytes = kj::mv(bytes)] - (jsg::Lock& js, size_t amount) mutable { - // Although we have a captured reference to the ioContext already, - // we should not assume that it is still valid here. Let's just grab - // IoContext::current() to move things along. - auto& ioContext = IoContext::current(); - if (amount == 0) { - return end ? - ioContext.awaitIo(output.end(), []() {}) : - js.resolvedPromise(); - } - return ioContext.awaitIo(js, output.write(bytes.begin(), amount), - [this, &output, end, bytes = kj::mv(bytes)] (jsg::Lock& js) mutable { - return pipeLoop(js, output, end, kj::mv(bytes)); - }); - }); -} - -// ====================================================================================== - -// TODO(stream-tee): Everything ReadableStreamJsTeeSource goes away. - -void ReadableStreamJsTeeSource::cancel(kj::Exception reason) { - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) { return; } - KJ_CASE_ONEOF(errored, kj::Exception) { kj::throwFatalException(kj::cp(errored)); } - KJ_CASE_ONEOF(readable, Readable) { - // For the tee adapter, the only thing we need to do here is - // reject and clear our own pending read, which is handled by - // when calling detach with an exception. The tee adapter will - // handle cleaning up the underlying controller when necessary - // to do so. - ioContext.addTask(ioContext.run( - [this, reason = kj::mv(reason)](Worker::Lock&) { - detach(kj::mv(reason), nullptr); - KJ_ASSERT(pendingRead == nullptr); - })); - } - } -} - -void ReadableStreamJsTeeSource::detach( - kj::Maybe maybeException, - kj::Maybe maybeJs) { - KJ_IF_MAYBE(controller, teeController) { - controller->removeBranch(this, maybeJs); - teeController = nullptr; - } - KJ_IF_MAYBE(exception, maybeException) { - KJ_IF_MAYBE(js, maybeJs) { - KJ_IF_MAYBE(read, pendingRead) { - read->resolver.reject(js->exceptionToJs(kj::cp(*exception)).getHandle(js->v8Isolate)); - pendingRead = nullptr; - } - } - state.init(kj::mv(*exception)); - } else { - // When maybeJs is nullptr, we are detaching while there is no isolate lock held. - // We only want to resolve the read promise and clear the pendingRead while we - // are within the isolate lock. - if (maybeJs != nullptr) { - KJ_IF_MAYBE(read, pendingRead) { - read->resolver.resolve(0); - pendingRead = nullptr; - } - } - state.init(); - } -} - -void ReadableStreamJsTeeSource::doClose() { - KJ_IF_MAYBE(read, pendingRead) { - read->resolver.resolve(0); - pendingRead = nullptr; - } - state.init(); -} - -void ReadableStreamJsTeeSource::doError(jsg::Lock& js, v8::Local reason) { - detach(js.exceptionToKj(js.v8Ref(reason)), js); -} - -void ReadableStreamJsTeeSource::handleData(jsg::Lock& js, ReadResult result) { - KJ_IF_MAYBE(read, pendingRead) { - // Make sure the pendingRead hasn't been canceled. If it has, we're just going to clear - // it and buffer the data. - KJ_IF_MAYBE(value, result.value) { - auto handle = value->getHandle(js); - if (!handle->IsArrayBufferView() && !handle->IsArrayBuffer()) { - auto reason = js.v8TypeError("This ReadableStream did not not return bytes."_kj); - read->resolver.reject(reason); - detach(js.exceptionToKj(js.v8Ref(reason)), js); - pendingRead = nullptr; - return; + const auto step = [&] { + return internalTryRead(js, bytes.begin(), 1, bytes.size()) + .then(js, [this, &output, end, bytes = kj::mv(bytes)] + (jsg::Lock& js, size_t amount) mutable { + // Although we have a captured reference to the ioContext already, + // we should not assume that it is still valid here. Let's just grab + // IoContext::current() to move things along. + auto& ioContext = IoContext::current(); + if (amount == 0) { + return end ? ioContext.awaitIo(output.end(), []() {}) : js.resolvedPromise(); } - - jsg::BufferSource source(js, handle); - auto ptr = source.asArrayPtr(); - // If we got too much data back, fulfill the remaining read and buffer the - // rest in the queue. - if (ptr.size() > read->bytes.size() - read->filled) { - auto bytesToCopy = read->bytes.size() - read->filled; - memcpy(read->bytes.begin() + read->filled, ptr.begin(), bytesToCopy); - std::copy(ptr.begin() + bytesToCopy, ptr.end(), std::back_inserter(queue)); - read->filled += bytesToCopy; - read->resolver.resolve(kj::cp(read->filled)); - pendingRead = nullptr; - return; - } - - // Otherwise, copy what we got into the read. - KJ_ASSERT(ptr.size() <= read->bytes.size() - read->filled); - memcpy(read->bytes.begin() + read->filled, ptr.begin(), ptr.size()); - read->filled += ptr.size(); - - // If we've filled up to or beyond the minBytes, we're done! Fulfill - // the promise, clear the pending read, and return. - if (read->filled >= read->minBytes) { - read->resolver.resolve(kj::cp(read->filled)); - pendingRead = nullptr; - return; - } - - // We have not yet met the minimum byte requirements, so we keep - // the current pending read in place, adjust the remaining minBytes - // down and call ensurePulling again. - read->minBytes -= ptr.size(); - KJ_ASSERT_NONNULL(teeController).ensurePulling(js); - return; - } - - KJ_ASSERT(result.done); - read->resolver.resolve(0); - pendingRead = nullptr; - } - - // If there is no waiting pending read, then we're just going to queue the bytes. - // If bytes were not returned, then transition to an errored state. - - KJ_IF_MAYBE(value, result.value) { - auto handle = value->getHandle(js); - if (!handle->IsArrayBufferView() && !handle->IsArrayBuffer()) { - detach(JSG_KJ_EXCEPTION(FAILED, TypeError, "This ReadableStream did not return bytes."), js); - return; - } - jsg::BufferSource source(js, handle); - auto ptr = source.asArrayPtr(); - std::copy(ptr.begin(), ptr.end(), std::back_inserter(queue)); - return; - } - - KJ_ASSERT(result.done); - detach(nullptr, nullptr); -} - -kj::Promise ReadableStreamJsTeeSource::tryRead( - void* buffer, - size_t minBytes, - size_t maxBytes) { - return ioContext.run([this, buffer, minBytes, maxBytes](Worker::Lock& lock) { - jsg::Lock& js = lock; - // Of particular note here: Notice that we attach a reference to this and the controller - // if it exists. This is to ensure that both the kj and js heap objects are live until - // the promise resolves. - auto promise = ioContext.awaitJs(internalTryRead(js, buffer, minBytes, maxBytes)) - .attach(kj::addRef(*this)); - KJ_IF_MAYBE(readable, state.tryGet()) { - promise = promise.attach(readable->addRef()); - } - return kj::mv(promise); - }); -} - -jsg::Promise ReadableStreamJsTeeSource::internalTryRead( - jsg::Lock& js, - void* buffer, - size_t minBytes, - size_t maxBytes) { - auto bytes = static_cast(buffer); - auto ptr = kj::ArrayPtr(bytes, maxBytes); + return ioContext.awaitIo(js, output.write(bytes.begin(), amount), + [this, &output, end, bytes = kj::mv(bytes)] (jsg::Lock& js) mutable { + return pipeLoop(js, output, end, kj::mv(bytes)); + }); + }); + }; KJ_SWITCH_ONEOF(state) { KJ_CASE_ONEOF(closed, StreamStates::Closed) { - if (queue.size() > 0) { - // There's still data in the queue. Copy it out until the queue is empty. - auto bytesToCopy = kj::min(maxBytes, queue.size()); - std::copy(queue.begin(), - queue.begin() + bytesToCopy, - ptr.begin()); - queue.erase(queue.begin(), queue.begin() + bytesToCopy); - return js.resolvedPromise(kj::cp(bytesToCopy)); - } - return js.resolvedPromise((size_t)0); + return js.resolvedPromise(); } KJ_CASE_ONEOF(errored, kj::Exception) { - return js.rejectedPromise(js.exceptionToJs(kj::cp(errored))); + return js.rejectedPromise(js.exceptionToJs(kj::cp(errored))); } - KJ_CASE_ONEOF(readable, Readable) { - if (pendingRead != nullptr) { - return js.rejectedPromise(js.v8TypeError("There is already a read pending."_kj)); - } - - if (queue.size() >= minBytes) { - // Good news, we can fulfill the minimum requirements of this tryRead - // synchronously from the queue. - // If there is any data at all in the queue, this is going to be the - // most likely path taken since we typically pass minBytes = 1. - auto bytesToCopy = kj::min(maxBytes, queue.size()); - std::copy(queue.begin(), - queue.begin() + bytesToCopy, - ptr.begin()); - queue.erase(queue.begin(), queue.begin() + bytesToCopy); - return js.resolvedPromise(kj::cp(bytesToCopy)); - } - - auto bytesToCopy = queue.size(); - if (bytesToCopy > 0) { - // This branch is unlikely to be taken unless we pass minBytes > 1. - // Otherwise, if the queue has any data at all and minBytes =1 , - // the above queue.size() >= minBytes path would be taken. - KJ_ASSERT(bytesToCopy < minBytes); - std::copy(queue.begin(), - queue.begin() + bytesToCopy, - ptr.begin()); - queue.clear(); - bytes += bytesToCopy; - minBytes -= bytesToCopy; - maxBytes -= bytesToCopy; - KJ_ASSERT(minBytes >= 1); - } - - auto prp = js.newPromiseAndResolver(); - pendingRead = PendingRead { - .resolver = kj::mv(prp.resolver), - .bytes = kj::ArrayPtr(bytes, maxBytes), - .minBytes = minBytes, - .filled = bytesToCopy, - }; - - KJ_ASSERT_NONNULL(teeController).ensurePulling(js); - - return prp.promise.catch_(js, [this](jsg::Lock& js, jsg::Value reason) mutable -> size_t { - state.init(js.exceptionToKj(reason.addRef(js))); - js.throwException(kj::mv(reason)); - }); + KJ_CASE_ONEOF(consumer, kj::Own) { + return step(); } - } - KJ_UNREACHABLE; -} - -kj::Promise> ReadableStreamJsTeeSource::pumpTo( - WritableStreamSink& output, bool end) { - // Here, the IoContext has to remain live throughout the entire - // pipe operation, so our deferred proxy will be a non-op. - return addNoopDeferredProxy(ioContext.run([this, &output, end](Worker::Lock& lock) { - jsg::Lock& js = lock; - // Of particular note here: Notice that we attach a reference to this and the controller - // if it exists. This is to ensure that both the kj and js heap objects are live until - // the promise resolves. - auto promise = ioContext.awaitJs(pipeLoop(js, output, end, - kj::heapArray(4096))) - .attach(kj::addRef(*this)); - KJ_IF_MAYBE(readable, state.tryGet()) { - promise = promise.attach(readable->addRef()); + KJ_CASE_ONEOF(consumer, kj::Own) { + return step(); } - return kj::mv(promise); - })); -} + } -jsg::Promise ReadableStreamJsTeeSource::pipeLoop( - jsg::Lock& js, - WritableStreamSink& output, - bool end, - kj::Array bytes) { - return internalTryRead(js, bytes.begin(), 1, bytes.size()) - .then(js, [this, &output, end, bytes = kj::mv(bytes)] - (jsg::Lock& js, size_t amount) mutable -> jsg::Promise { - // Although we have captured reference to ioContext here, - // we should not assume that the reference is still valid in - // the continuation. Let's just grab IoContext::current() - // to move things along. - auto& ioContext = IoContext::current(); - if (amount == 0) { - return end ? - ioContext.awaitIo(output.end(), []{}) : - js.resolvedPromise(); - } - return ioContext.awaitIo(js, output.write(bytes.begin(), amount), - [this, &output, end, bytes = kj::mv(bytes)] (jsg::Lock& js) mutable { - return pipeLoop(js, output, end, kj::mv(bytes)); - }); - }); + KJ_UNREACHABLE; } // ====================================================================================== @@ -3397,6 +2417,7 @@ jsg::Promise WritableStreamDefaultController::write( return impl.write(js, JSG_THIS, value); } +// ====================================================================================== WritableStreamJsController::WritableStreamJsController() {} WritableStreamJsController::WritableStreamJsController(StreamStates::Closed closed) @@ -3598,7 +2619,7 @@ kj::Maybe> WritableStreamJsController::tryPipeFrom( // Let's also acquire the destination pipe lock. lock.pipeLock(KJ_ASSERT_NONNULL(owner), kj::mv(source), options); - return pipeLoop(js); + return pipeLoop(js).then(js, JSG_VISITABLE_LAMBDA((ref = addRef()), (ref), (auto& js) {})); } jsg::Promise WritableStreamJsController::pipeLoop(jsg::Lock& js) { @@ -3688,9 +2709,8 @@ jsg::Promise WritableStreamJsController::pipeLoop(jsg::Lock& js) { // we call pipeLoop again to move on to the next iteration. return pipeLock.source.read(js).then(js, - JSG_VISITABLE_LAMBDA((this, preventCancel, pipeThrough, &source, ref = addRef()), - (ref), (jsg::Lock& js, ReadResult result) -> jsg::Promise { - + [this, preventCancel, pipeThrough, &source] + (jsg::Lock& js, ReadResult result) -> jsg::Promise { auto& pipeLock = lock.getPipe(); KJ_IF_MAYBE(promise, pipeLock.checkSignal(js, *this)) { @@ -3716,7 +2736,7 @@ jsg::Promise WritableStreamJsController::pipeLoop(jsg::Lock& js) { } return rejectedMaybeHandledPromise(js, reason, pipeThrough); }); - }), [this] (jsg::Lock& js, jsg::Value value) { + }, [this] (jsg::Lock& js, jsg::Value value) { // The read failed. We will handle the error at the start of the next iteration. return pipeLoop(js); }); @@ -3750,7 +2770,7 @@ jsg::Promise WritableStreamJsController::write( return js.rejectedPromise(errored.addRef(js)); } KJ_CASE_ONEOF(controller, Controller) { - return controller->write(js, value.orDefault(js.v8Undefined())); + return controller->write(js, value.orDefault([&] { return js.v8Undefined(); })); } } KJ_UNREACHABLE; @@ -3932,6 +2952,10 @@ void TransformStreamDefaultController::init( KJ_ASSERT(maybeWritableController == nullptr); maybeWritableController = static_cast(writable->getController()); + // The TransformStreamDefaultController needs to have a reference to the underlying controller + // and not just the readable because if the readable is teed, or passed off to source, etc, + // the TransformStream has to make sure that it can continue to interface with the controller + // to push data into it. auto& readableController = static_cast(readable->getController()); auto readableRef = KJ_ASSERT_NONNULL(readableController.getController()); maybeReadableController = kj::mv(KJ_ASSERT_NONNULL( @@ -3939,6 +2963,9 @@ void TransformStreamDefaultController::init( auto transformer = kj::mv(maybeTransformer).orDefault({}); + // TODO(someday): The stream standard includes placeholders for supporting byte-oriented + // TransformStreams but does not yet define them. For now, we are limiting our implementation + // here to only support value-based transforms. JSG_REQUIRE(transformer.readableType == nullptr, TypeError, "transformer.readableType must be undefined."); JSG_REQUIRE(transformer.writableType == nullptr, TypeError, diff --git a/src/workerd/api/streams/standard.h b/src/workerd/api/streams/standard.h index 9a9463ba32f..7688ecd72c3 100644 --- a/src/workerd/api/streams/standard.h +++ b/src/workerd/api/streams/standard.h @@ -47,6 +47,12 @@ struct UnderlyingSource { // to be value-oriented rather than byte-oriented. jsg::Optional autoAllocateChunkSize; + // Used only when type is equal to "bytes", the autoAllocateChunkSize defines + // the size of automatically allocated buffer that is created when a default + // mode read is performed on a byte-oriented ReadableStream that supports + // BYOB reads. The stream standard makes this optional to support and defines + // no default value. We've chosen to use a default value of 4096. If given, + // the value must be greater than zero. jsg::Optional> start; jsg::Optional> pull; @@ -117,15 +123,6 @@ struct Transformer { // // * ReadableStream -> ReadableStreamInternalController -> IoOwn // -// The ReadableStreamJsController implements two interfaces: -// * ReadableStreamController (which is the actual abstraction API, also implemented by -// ReadableStreamInternalController) -// * jscontroller::ReaderOwner -// -// jscontroller::ReaderOwner is an abstraction implemented by any object capable of owning -// the reference to a ReadableStreamDefaultController or ReadableByteStreamController and -// interacting with it. We'll talk about why this abstraction is necessary in a moment. -// // When user-code creates a JavaScript-backed ReadableStream using the `ReadableStream` // object constructor, they pass along an object called an "underlying source" that provides // JavaScript functions the ReadableStream will call to either initialize, close, or source @@ -183,31 +180,23 @@ struct Transformer { // and fully consume the stream entirely from within JavaScript without ever engaging the kj event // loop. // -// When you tee() a JavaScript-backed ReadableStream, the stream is put into a TeeLocked state. -// The newly created ReadableStream branches wrap ReadableStreamJsTeeController instances that -// each share a reference to the original tee'd ReadableStream that owns the underlying -// controller and interact with it via the TeeController API. +// When you tee() a JavaScript-backed ReadableStream, the stream is put into a locked state and +// the data is funneled out through two separate "branches" (two new `ReadableStream`s). // -// When anything reads from a tee branch, the tee controller is asked to read from the underlying -// source. When the underlying source responds to the tee controller's read request, the -// tee adapter forwards the read result on to all of the branches. +// When anything reads from a tee branch, the underlying controller is asked to read from the +// underlying source. When the underlying source responds to that read request, the +// data is forwarded to all of the known branches. // // All of this works great from within JavaScript, but what about when you want to use a // JavaScript-backed ReadableStream to respond to a fetch request? Or interface it at all // with any of the existing internal streams that are based on the older ReadableStreamSource -// API. For those cases, ReadableStreamJsController and ReadableStreamJsTeeController each -// implement the `removeSource()` method to acquire a `ReadableStreamSource` that wraps the -// JavaScript controller. -// -// kj::Own -> jsg::Ref -// kj::Own -> jsg::Ref -// kj::Own -> kj::Own +// API. For those cases, ReadableStreamJsController implements the `removeSource()` method to +// acquire a `ReadableStreamJsSource` that wraps the JavaScript controller. // -// Each of these implement the older ReadableStreamSource API. The ReadableStreamJsSource -// also implements the jscontroller::ReaderOwner interface. +// The `ReadableStreamJsSource` implements the internal ReadableStreamSource API. // -// Whenever tryRead is invoked on either type of source, it will attempt to acquire an -// isolate lock within which it will interface with the JavaScript-backed underlying controller. +// Whenever tryRead is invoked this source, it will attempt to acquire an isolate lock within +// which it will interface with the JavaScript-backed underlying controller. // Value streams can be used only so long as the only values they pass along happen to be // interpretable as bytes (so ArrayBufferViews and ArrayBuffers). These support the minimal // contract of tryRead including support for the minBytes argument, performing multiple reads @@ -245,131 +234,25 @@ struct Transformer { // All write operations on a JavaScript-backed WritableStream are processed within the // isolate lock using JavaScript promises instead of kj::Promises. +struct ValueReadable; +struct ByteReadable; +KJ_DECLARE_NON_POLYMORPHIC(ValueReadable); +KJ_DECLARE_NON_POLYMORPHIC(ByteReadable); namespace jscontroller { // The jscontroller namespace defines declarations that are common to all of the the // JavaScript-backed ReadableStream and WritableStream variants. -using ReadRequest = jsg::Promise::Resolver; -using WriteRequest = jsg::Promise::Resolver; using CloseRequest = jsg::Promise::Resolver; using DefaultController = jsg::Ref; using ByobController = jsg::Ref; -//------------------------------ -struct ByteQueueEntry; -struct ValueQueueEntry; -struct ByteQueueEntry { - // Used by the template class Queue (below) to implement a byte-queue - // used by the ReadableByteStreamController. - - jsg::BackingStore store; - - static size_t getSize(ByteQueueEntry& type) { return type.store.size(); } - - static void visitForGc(jsg::GcVisitor& visitor, ByteQueueEntry& type) {} -}; - -struct ValueQueueEntry { - // Used by class Queue (below) to implement a JavaScript value queue - // used by the ReadableStreamDefaultController and WritableStreamDefaultController. - // Each entry consists of some arbitrary JavaScript value and a size that is - // calculated by the size callback function provided in the stream constructor. - - jsg::Value value; - size_t size; - - static size_t getSize(ValueQueueEntry& type) { return type.size; } - - static void visitForGc(jsg::GcVisitor& visitor, ValueQueueEntry& type) { - visitor.visit(type.value); - } -}; - -template -class Queue { - // Encapsulates a deque used to manage the internal queue of a - // JavaScript-backed stream. Really just a convenience utility - // that reduces and encapsulates some of the boilerplate code. -public: - struct Close { - // A sentinel object used to identify that no additional - // data will be written to the queue. - }; - - explicit Queue() = default; - Queue(Queue&& other) = default; - Queue& operator=(Queue&& other) = default; - - void push(T entry) { - KJ_ASSERT(entries.empty() || !entries.back().template is()); - queueTotalSize += T::getSize(entry); - entries.push_back(kj::mv(entry)); - } - - void close() { - KJ_ASSERT(entries.empty() || !entries.back().template is()); - entries.push_back(Close {}); - } - - size_t size() const { return queueTotalSize; } - - bool empty() const { return entries.empty(); } - - void reset() { - entries.clear(); - queueTotalSize = 0; - } - - template - Type pop() { - KJ_ASSERT(!entries.empty()); - auto entry = kj::mv(entries.front()); - KJ_IF_MAYBE(e, entry.template tryGet()) { - queueTotalSize -= T::getSize(*e); - } - entries.pop_front(); - return kj::mv(entry.template get()); - } - - T& peek() { - KJ_ASSERT(!entries.empty()); - return entries.front().template get(); - } - - bool frontIsClose() { - KJ_ASSERT(!entries.empty()); - return entries.front().template is(); - } - - void dec(size_t size) { - KJ_ASSERT(queueTotalSize >= size); - queueTotalSize -= size; - } - - void visitForGc(jsg::GcVisitor& visitor) { - for (auto& entry : entries) { - KJ_IF_MAYBE(e, entry.template tryGet()) { - T::visitForGc(visitor, *e); - } - } - } - -private: - std::deque> entries; - size_t queueTotalSize = 0; - // Either the total number of bytes or the total number of values. -}; - -using ByteQueue = Queue; -using ValueQueue = Queue; - -// ------------------------------ +// ======================================================================================= // ReadableStreams can be either Closed, Errored, or Readable. // WritableStreams can be either Closed, Errored, Erroring, or Writable. -struct Readable {}; + struct Writable {}; -// ------------------------------ +// ======================================================================================= // The Unlocked, Locked, ReaderLocked, and WriterLocked structs // are used to track the current lock status of JavaScript-backed streams. // All readable and writable streams begin in the Unlocked state. When a @@ -384,18 +267,14 @@ struct Writable {}; // When either the removeSource() or removeSink() methods are called, the streams // will transition to the Locked state. // -// When a ReadableStreamJsController is tee()'d, it will enter the TeeLocked state. -// The TeeLocked struct is defined within the ReadableLockImpl class below. -// When a ReadableStreamJsTeeController is tee()'d, the Locked state is used since -// the tee controller does not need the full TeeLocked function. +// When a ReadableStreamJsController is tee()'d, it will enter the locked state. template class ReadableLockImpl { - // A utility class used by ReadableStreamJsController and ReadableStreamJsTeeController + // A utility class used by ReadableStreamJsController // for implementing the reader lock in a consistent way (without duplicating any code). public: using PipeController = ReadableStreamController::PipeController; - using TeeController = ReadableStreamController::TeeController; using Reader = ReadableStreamController::Reader; bool isLockedToReader() const { return !state.template is(); } @@ -405,12 +284,13 @@ class ReadableLockImpl { void releaseReader(Controller& self, Reader& reader, kj::Maybe maybeJs); // See the comment for releaseReader in common.h for details on the use of maybeJs + void onClose(); + void onError(jsg::Lock& js, v8::Local reason); + kj::Maybe tryPipeLock( Controller& self, jsg::Ref destination); - kj::Maybe tryTeeLock(Controller& self); - void visitForGc(jsg::GcVisitor& visitor); private: @@ -429,7 +309,9 @@ class ReadableLockImpl { } void cancel(jsg::Lock& js, v8::Local reason) override { - inner.doCancel(js, reason); + // Cancel here returns a Promise but we do not need to propagate it. + // We can safely drop it on the floor here. + auto promise KJ_UNUSED = inner.cancel(js, reason); } void close() override { @@ -461,49 +343,7 @@ class ReadableLockImpl { friend Controller; }; - class TeeLocked: public TeeController { - public: - explicit TeeLocked(Controller& inner) - : inner(inner) {} - - TeeLocked(TeeLocked&& other) = default; - - ~TeeLocked() override {} - - void addBranch(Branch* branch) override; - - void close() override; - - void error(jsg::Lock& js, v8::Local reason) override; - - void ensurePulling(jsg::Lock& js) override; - - void removeBranch(Branch* branch, kj::Maybe maybeJs) override; - // See the comment for removeBranch in common.h for details on the use of maybeJs - - void visitForGc(jsg::GcVisitor& visitor); - - private: - jsg::Promise pull(jsg::Lock& js); - - void forEachBranch(auto func) { - // A branch can delete itself while handling the func which will - // invalidate the iterator so we create a copy and iterate that - // instead. - kj::Vector pending; - for (auto& branch : branches) { pending.add(branch); } - for (auto& branch : pending) { - func(branch); - } - } - - Controller& inner; - bool pullAgain = false; - kj::Maybe> maybePulling; - kj::HashSet branches; - }; - - kj::OneOf state = Unlocked(); + kj::OneOf state = Unlocked(); friend Controller; }; @@ -552,28 +392,7 @@ class WritableLockImpl { friend Controller; }; -// ------------------------------ -class ReaderOwner { - // The ReaderOwner is the current owner of a ReadableStreamDefaultController - // or ReadableByteStreamController. This can be one of either a - // ReadableStreamJsController or ReadableStreamJsSource. The ReaderOwner interface - // allows the underlying controller to communicate status updates up to the current - // owner without caring about what kind of thing the owner currently is. -public: - virtual void doClose() = 0; - // Communicate to the owner that the stream has been closed. The owner should release - // ownership of the underlying controller and allow it to be garbage collected as soon - // as possible. - - virtual void doError(jsg::Lock& js, v8::Local reason) = 0; - // Communicate to the owner that the stream has been errored. The owner should remember - // the error reason, and release ownership of the underlying controller and allow it to - // be garbage collected as soon as possible. - - virtual bool isLocked() const = 0; - virtual bool isLockedReaderByteOriented() = 0; -}; - +// ======================================================================================= class WriterOwner { // The WriterOwner is the current owner of a WritableStreamDefaultcontroller. // Currently, this can only be a WritableStreamJsController. @@ -598,52 +417,47 @@ class WriterOwner { virtual void maybeRejectReadyPromise(jsg::Lock& js, v8::Local reason) = 0; }; -// ------------------------------ +// ======================================================================================= template class ReadableImpl { // The ReadableImpl provides implementation that is common to both the // ReadableStreamDefaultController and the ReadableByteStreamController. public: - ReadableImpl(ReaderOwner& owner) : owner(owner) {} + using Consumer = typename Self::QueueType::Consumer; + using Entry = typename Self::QueueType::Entry; + using StateListener = typename Self::QueueType::ConsumerImpl::StateListener; + + ReadableImpl(UnderlyingSource underlyingSource, + StreamQueuingStrategy queuingStrategy); + + void start(jsg::Lock& js, jsg::Ref self); jsg::Promise cancel(jsg::Lock& js, jsg::Ref self, v8::Local maybeReason); - void setup( - jsg::Lock& js, - jsg::Ref self, - UnderlyingSource underlyingSource, - StreamQueuingStrategy queuingStrategy); - bool canCloseOrEnqueue(); - ReadRequest dequeueReadRequest(); - void doCancel(jsg::Lock& js, jsg::Ref self, v8::Local reason); + void close(jsg::Lock& js); + + void enqueue(jsg::Lock& js, kj::Own entry, jsg::Ref self); + void doClose(jsg::Lock& js); - void doError(jsg::Lock& js, v8::Local reason); + void doError(jsg::Lock& js, jsg::Value reason); kj::Maybe getDesiredSize(); void pullIfNeeded(jsg::Lock& js, jsg::Ref self); - void resolveReadRequest( - ReadResult result, - kj::Maybe maybeRequest = nullptr); - - void setOwner(kj::Maybe owner) { - this->owner = owner; - } - - ReaderOwner& getOwner() { - return JSG_REQUIRE_NONNULL(owner, TypeError, "This stream has been closed."); - } + bool hasPendingReadRequests(); bool shouldCallPull(); + kj::Own getConsumer(kj::Maybe listener); + void visitForGc(jsg::GcVisitor& visitor); private: @@ -652,11 +466,17 @@ class ReadableImpl { kj::Maybe> pulling; kj::Maybe> canceling; + kj::Maybe> start; kj::Maybe> pull; kj::Maybe> cancel; kj::Maybe> size; - Algorithms() {}; + Algorithms(UnderlyingSource underlyingSource, StreamQueuingStrategy queuingStrategy) + : start(kj::mv(underlyingSource.start)), + pull(kj::mv(underlyingSource.pull)), + cancel(kj::mv(underlyingSource.cancel)), + size(kj::mv(queuingStrategy.size)) {} + Algorithms(Algorithms&& other) = default; Algorithms& operator=(Algorithms&& other) = default; @@ -664,23 +484,22 @@ class ReadableImpl { starting = nullptr; pulling = nullptr; canceling = nullptr; + start = nullptr; pull = nullptr; cancel = nullptr; size = nullptr; } void visitForGc(jsg::GcVisitor& visitor) { - visitor.visit(starting, pulling, canceling, pull, cancel, size); + visitor.visit(starting, pulling, canceling, start, pull, cancel, size); } }; using Queue = typename Self::QueueType; - kj::Maybe owner; - kj::OneOf state = Readable(); + kj::OneOf state; Algorithms algorithms; - Queue queue; - std::deque readRequests; + bool closeRequested = false; bool disturbed = false; bool pullAgain = false; @@ -706,6 +525,16 @@ class WritableImpl { public: using PendingAbort = WritableStreamController::PendingAbort; + struct WriteRequest { + jsg::Promise::Resolver resolver; + jsg::Value value; + size_t size; + + void visitForGc(jsg::GcVisitor& visitor) { + visitor.visit(resolver, value); + } + }; + WritableImpl(WriterOwner& owner); jsg::Promise abort(jsg::Lock& js, @@ -799,8 +628,6 @@ class WritableImpl { } }; - using Queue = typename Self::QueueType; - kj::Maybe owner; jsg::Ref signal; kj::OneOf writeRequests; + size_t amountBuffered = 0; kj::Maybe inFlightWrite; kj::Maybe inFlightClose; @@ -822,95 +650,41 @@ class WritableImpl { friend Self; }; -struct ValueReadable final { - DefaultController controller; - kj::Own consumer; - - ValueReadable(DefaultController controller); - KJ_DISALLOW_COPY(ValueReadable); - - kj::Own clone(jsg::Lock& js); - // A single ReadableStreamDefaultController can have multiple consumers. - // When the ValueReadable constructor is used, the new consumer is added - // and starts to receive new data that becomes enqueued. When clone - // is used, any state currently held by this consumer is copied to the - // new consumer. - - jsg::Promise cancel(jsg::Lock& js, jsg::Optional> maybeReason); - // When a ReadableStream is canceled, the expected behavior is that the underlying - // controller is notified and the cancel algorithm on the underlying source is - // called. When there are multiple ReadableStreams sharing consumption of a - // controller, however, it should act as a shared pointer of sorts, canceling - // the underlying controller only when the last reader is canceled. -}; - -struct ByteReadable final { - ByobController controller; - kj::Own consumer; - - ByteReadable(DefaultController controller); - KJ_DISALLOW_COPY(ByteReadable); - - kj::Own clone(jsg::Lock& js); - // A single ReadableByteStreamController can have multiple consumers. - // When the ByteReadable constructor is used, the new consumer is added - // and starts to receive new data that becomes enqueued. When clone - // is used, any state currently held by this consumer is copied to the - // new consumer. - - jsg::Promise cancel(jsg::Lock& js, jsg::Optional> maybeReason); - // When a ReadableStream is canceled, the expected behavior is that the underlying - // controller is notified and the cancel algorithm on the underlying source is - // called. When there are multiple ReadableStreams sharing consumption of a - // controller, however, it should act as a shared pointer of sorts, canceling - // the underlying controller only when the last reader is canceled. -}; } // namespace jscontroller +// ======================================================================================= + class ReadableStreamDefaultController: public jsg::Object { // ReadableStreamDefaultController is a JavaScript object defined by the streams specification. // It is capable of streaming any JavaScript value through it, including typed arrays and // array buffers, but treats all values as opaque. BYOB reads are not supported. public: - using QueueType = jscontroller::ValueQueue; - using ReaderOwner = jscontroller::ReaderOwner; - using ReadRequest = jscontroller::ReadRequest; + using QueueType = ValueQueue; using ReadableImpl = jscontroller::ReadableImpl; - ReadableStreamDefaultController(ReaderOwner& owner); + ReadableStreamDefaultController(UnderlyingSource underlyingSource, + StreamQueuingStrategy queuingStrategy); + + void start(jsg::Lock& js); jsg::Promise cancel(jsg::Lock& js, - jsg::Optional> maybeReason); + jsg::Optional> maybeReason); void close(jsg::Lock& js); - void doCancel(jsg::Lock& js, v8::Local reason); - - inline bool canCloseOrEnqueue() { return impl.canCloseOrEnqueue(); } - inline bool hasBackpressure() { return !impl.shouldCallPull(); } - - void enqueue(jsg::Lock& js, jsg::Optional> chunk); - - void doEnqueue(jsg::Lock& js, jsg::Optional> chunk); - - void error(jsg::Lock& js, v8::Local reason); - + bool canCloseOrEnqueue(); + bool hasBackpressure(); kj::Maybe getDesiredSize(); - bool hasPendingReadRequests(); - void pull(jsg::Lock& js, ReadRequest readRequest); + void enqueue(jsg::Lock& js, jsg::Optional> chunk); - jsg::Promise read(jsg::Lock& js); + void error(jsg::Lock& js, v8::Local reason); - void setOwner(kj::Maybe owner); + void pull(jsg::Lock& js); - ReaderOwner& getOwner() { return impl.getOwner(); } - - void setup( - jsg::Lock& js, - UnderlyingSource underlyingSource, - StreamQueuingStrategy queuingStrategy); + kj::Own getConsumer( + kj::Maybe stateListener); JSG_RESOURCE_TYPE(ReadableStreamDefaultController) { JSG_READONLY_INSTANCE_PROPERTY(desiredSize, getDesiredSize); @@ -922,9 +696,7 @@ class ReadableStreamDefaultController: public jsg::Object { private: ReadableImpl impl; - void visitForGc(jsg::GcVisitor& visitor) { - visitor.visit(impl); - } + void visitForGc(jsg::GcVisitor& visitor); }; class ReadableStreamBYOBRequest: public jsg::Object { @@ -943,9 +715,13 @@ class ReadableStreamBYOBRequest: public jsg::Object { // object name. public: ReadableStreamBYOBRequest( - jsg::V8Ref view, - jsg::Ref controller, - size_t atLeast); + jsg::Lock& js, + kj::Own readRequest, + jsg::Ref controller); + + KJ_DISALLOW_COPY(ReadableStreamBYOBRequest); + ReadableStreamBYOBRequest(ReadableStreamBYOBRequest&&) = delete; + ReadableStreamBYOBRequest& operator=(ReadableStreamBYOBRequest&&) = delete; kj::Maybe getAtLeast(); // getAtLeast is a non-standard Workers-specific extension that specifies @@ -972,12 +748,16 @@ class ReadableStreamBYOBRequest: public jsg::Object { private: struct Impl { - jsg::V8Ref view; + kj::Own readRequest; jsg::Ref controller; - size_t atLeast; - Impl(jsg::V8Ref view, - jsg::Ref controller, - size_t atLeast); + jsg::V8Ref view; + + Impl(jsg::Lock& js, + kj::Own readRequest, + jsg::Ref controller) + : readRequest(kj::mv(readRequest)), + controller(kj::mv(controller)), + view(js.v8Ref(this->readRequest->getView(js))) {} }; kj::Maybe maybeImpl; @@ -990,54 +770,34 @@ class ReadableByteStreamController: public jsg::Object { // It is capable of only streaming byte data through it in the form of typed arrays. // BYOB reads are supported. public: - using QueueType = jscontroller::ByteQueue; - using ReadRequest = jscontroller::ReadRequest; - using ReaderOwner = jscontroller::ReaderOwner; + using QueueType = ByteQueue; using ReadableImpl = jscontroller::ReadableImpl; - struct PendingPullInto { - jsg::BackingStore store; - size_t filled; - size_t atLeast; - enum class Type { DEFAULT, BYOB } type; - }; + ReadableByteStreamController(UnderlyingSource underlyingSource, + StreamQueuingStrategy queuingStrategy); - ReadableByteStreamController(ReaderOwner& owner); + void start(jsg::Lock& js); jsg::Promise cancel(jsg::Lock& js, jsg::Optional> maybeReason); void close(jsg::Lock& js); - void doCancel(jsg::Lock& js, v8::Local reason); - void enqueue(jsg::Lock& js, jsg::BufferSource chunk); void error(jsg::Lock& js, v8::Local reason); - inline bool canCloseOrEnqueue() { return impl.canCloseOrEnqueue(); } - inline bool hasBackpressure() { return !impl.shouldCallPull(); } - - kj::Maybe> getByobRequest(jsg::Lock& js); - + bool canCloseOrEnqueue(); + bool hasBackpressure(); kj::Maybe getDesiredSize(); - bool hasPendingReadRequests(); - void pull(jsg::Lock& js, ReadRequest readRequest); - - jsg::Promise read(jsg::Lock& js, - kj::Maybe maybeByobOptions); - - void setOwner(kj::Maybe owner) { - impl.setOwner(owner); - } + kj::Maybe> getByobRequest(jsg::Lock& js); - ReaderOwner& getOwner() { return impl.getOwner(); } + void pull(jsg::Lock& js); - void setup(jsg::Lock& js, - UnderlyingSource underlyingSource, - StreamQueuingStrategy queuingStrategy); + kj::Own getConsumer( + kj::Maybe stateListener); JSG_RESOURCE_TYPE(ReadableByteStreamController) { JSG_READONLY_INSTANCE_PROPERTY(byobRequest, getByobRequest); @@ -1048,146 +808,16 @@ class ReadableByteStreamController: public jsg::Object { } private: - - void commitPullInto(jsg::Lock& js, PendingPullInto pullInto); - - PendingPullInto dequeuePendingPullInto(); - - bool fillPullInto(PendingPullInto& pullInto); - - bool isReadable() const; - - void pullIntoUsingQueue(jsg::Lock& js); - - void queueDrain(jsg::Lock& js); - - void respondInternal(jsg::Lock& js, size_t bytesWritten); - - size_t updatePullInto(jsg::Lock& js, jsg::BufferSource view); - ReadableImpl impl; kj::Maybe> maybeByobRequest; - size_t autoAllocateChunkSize = UnderlyingSource::DEFAULT_AUTO_ALLOCATE_CHUNK_SIZE; - std::deque pendingPullIntos; - void visitForGc(jsg::GcVisitor& visitor) { - visitor.visit(maybeByobRequest, impl); - } + void visitForGc(jsg::GcVisitor& visitor); friend class ReadableStreamBYOBRequest; friend class ReadableStreamJsController; }; -class ReadableStreamJsTeeController: public ReadableStreamController, - public ReadableStreamController::TeeController::Branch { - // The ReadableStreamJsTeeController backs ReadableStreams that have been teed off - // from a ReadableStreamJsController. Each instance is a branch registered with - // a shared TeeController that is responsible for coordinating the pull of data from the - // underlying ReadableStreamDefaultController or ReadableByteStreamController. - // - // Per the streams specification, ReadableStreamJsTeeController is *always* value-oriented, - // even if the underlying stream is byte-oriented. This means that tee branches will never - // support BYOB reads, but still may read from underlying byte sources. -public: - using ByobController = jscontroller::ByobController; - using DefaultController = jscontroller::DefaultController; - using Readable = jscontroller::Readable; - using ReadableLockImpl = jscontroller::ReadableLockImpl; - using ReadRequest = jscontroller::ReadRequest; - using TeeController = ReadableStreamController::TeeController; - using Queue = std::deque; - - struct Attached { - // Represents the state when the JSTeeController is attached to - // the inner TeeController. - jsg::Ref ref; - TeeController& controller; - - Attached(jsg::Ref ref, TeeController& controller); - }; - - explicit ReadableStreamJsTeeController( - jsg::Ref baseStream, - TeeController& teeController); - - explicit ReadableStreamJsTeeController( - jsg::Lock& js, - kj::Maybe attached, - Queue& queue); - - explicit ReadableStreamJsTeeController(ReadableStreamJsTeeController&& other); - - ~ReadableStreamJsTeeController() noexcept(false); - - jsg::Ref addRef() override; - - jsg::Promise cancel(jsg::Lock& js, - jsg::Optional> reason) override; - - void doClose() override; - - void doError(jsg::Lock& js, v8::Local reason) override; - - void handleData(jsg::Lock& js, ReadResult result) override; - - bool hasPendingReadRequests(); - - bool isByteOriented() const override; - - bool isClosedOrErrored() const override; - - bool isDisturbed() override; - - bool isLockedToReader() const override; - - bool lockReader(jsg::Lock& js, Reader& reader) override; - - jsg::Promise pipeTo( - jsg::Lock& js, - WritableStreamController& destination, - PipeToOptions options) override; - - kj::Maybe> read( - jsg::Lock& js, - kj::Maybe byobOptions) override; - - void releaseReader(Reader& reader, kj::Maybe maybeJs) override; - // See the comment for releaseReader in common.h for details on the use of maybeJs - - kj::Maybe> removeSource(jsg::Lock& js) override; - - void setOwnerRef(ReadableStream& owner) override; - - Tee tee(jsg::Lock& js) override; - - kj::Maybe tryPipeLock(jsg::Ref destination) override; - - void visitForGc(jsg::GcVisitor& visitor) override; - -private: - static Queue copyQueue(Queue& queue, jsg::Lock& js); - void detach(kj::Maybe maybeJs); - // See the comment for removeBranch in common.h for details on the use of maybeJs - void doCancel(jsg::Lock& js, v8::Local reason); - void drain(kj::Maybe> reason); - void finishClosing(jsg::Lock& js); - - kj::Maybe owner; - kj::OneOf state = StreamStates::Closed(); - kj::Maybe innerState; - ReadableLockImpl lock; - bool disturbed = false; - bool closePending = false; - - std::deque queue; - std::deque readRequests; - - friend ReadableLockImpl; - friend ReadableLockImpl::PipeLocked; -}; - -class ReadableStreamJsController: public ReadableStreamController, - public jscontroller::ReaderOwner { +class ReadableStreamJsController: public ReadableStreamController { // The ReadableStreamJsController provides the implementation of custom // ReadableStreams backed by a user-code provided Underlying Source. The implementation // is fairly complicated and defined entirely by the streams specification. @@ -1226,53 +856,48 @@ class ReadableStreamJsController: public ReadableStreamController, using DefaultController = jscontroller::DefaultController; using ReadableLockImpl = jscontroller::ReadableLockImpl; - explicit ReadableStreamJsController(); - - explicit ReadableStreamJsController(StreamStates::Closed closed); - - explicit ReadableStreamJsController(StreamStates::Errored errored); - + explicit ReadableStreamJsController() = default; ReadableStreamJsController(ReadableStreamJsController&& other) = default; ReadableStreamJsController& operator=(ReadableStreamJsController&& other) = default; - ~ReadableStreamJsController() noexcept(false) override { - // Ensure if the controller is still attached, it's c++ reference to this source is cleared. - // This can be the case, for instance, if the ReadableStream instance is garbage collected - // while there is still a reference to the controller being held somewhere. - detachFromController(); - } + explicit ReadableStreamJsController(StreamStates::Closed closed); + explicit ReadableStreamJsController(StreamStates::Errored errored); + explicit ReadableStreamJsController(jsg::Lock& js, ValueReadable& consumer); + explicit ReadableStreamJsController(jsg::Lock& js, ByteReadable& consumer); + explicit ReadableStreamJsController(kj::Own consumer); + explicit ReadableStreamJsController(kj::Own consumer); jsg::Ref addRef() override; + void setup( + jsg::Lock& js, + jsg::Optional maybeUnderlyingSource, + jsg::Optional maybeQueuingStrategy); + jsg::Promise cancel( jsg::Lock& js, jsg::Optional> reason) override; + // Signals that this ReadableStream is no longer interested in the underlying + // data source. Whether this cancels the underlying data source also depends + // on whether or not there are other ReadableStreams still attached to it. + // This operation is terminal. Once called, even while the returned Promise + // is still pending, the ReadableStream will be no longer usable and any + // data still in the queue will be dropped. Pending read requests will be + // rejected if a reason is given, or resolved with no data otherwise. - void doCancel(jsg::Lock& js, v8::Local reason); - - void controllerClose(jsg::Lock& js); - - void controllerError(jsg::Lock& js, v8::Local reason); - - void doClose() override; + void doClose(); - void doError(jsg::Lock& js, v8::Local reason) override; + void doError(jsg::Lock& js, v8::Local reason); bool canCloseOrEnqueue(); bool hasBackpressure(); - void defaultControllerEnqueue(jsg::Lock& js, v8::Local chunk); - bool isByteOriented() const override; bool isDisturbed() override; - bool isLocked() const override; - bool isClosedOrErrored() const override; - bool isLockedReaderByteOriented() override; - bool isLockedToReader() const override; bool lockReader(jsg::Lock& js, Reader& reader) override; @@ -1297,41 +922,27 @@ class ReadableStreamJsController: public ReadableStreamController, void setOwnerRef(ReadableStream& stream) override; - void setup( - jsg::Lock& js, - jsg::Optional maybeUnderlyingSource, - jsg::Optional maybeQueuingStrategy); - Tee tee(jsg::Lock& js) override; kj::Maybe tryPipeLock(jsg::Ref destination) override; void visitForGc(jsg::GcVisitor& visitor) override; - inline kj::Maybe> getController() { - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(closed, StreamStates::Closed) { return nullptr; } - KJ_CASE_ONEOF(errored, StreamStates::Errored) { return nullptr; } - KJ_CASE_ONEOF(controller, DefaultController) { - return kj::Maybe(controller.addRef()); - } - KJ_CASE_ONEOF(controller, ByobController) { - return kj::Maybe(controller.addRef()); - } - } - KJ_UNREACHABLE; - } + kj::Maybe> getController(); private: bool hasPendingReadRequests(); - void detachFromController(); kj::Maybe owner; + kj::OneOf state = StreamStates::Closed(); + kj::Own, + kj::Own> state = StreamStates::Closed(); + ReadableLockImpl lock; + // The lock state is separate because a closed or errored stream can still be locked. + kj::Maybe> maybeTransformer; bool disturbed = false; @@ -1340,14 +951,12 @@ class ReadableStreamJsController: public ReadableStreamController, }; class ReadableStreamJsSource: public kj::Refcounted, - public ReadableStreamSource, - public jscontroller::ReaderOwner { + public ReadableStreamSource { // The ReadableStreamJsSource is a bridge between the JavaScript-backed // streams and the existing native internal streams. When an instance is - // retrieved from the ReadableStreamJavaScriptController, it takes over - // ownership of the ReadableStreamDefaultController or ReadableByteStreamController - // and takes over all interaction with them. It will ensure that the callbacks on - // the Underlying Stream are called correctly. + // retrieved from the ReadableStreamJsController, it takes over ownership of the + // ReadableStreamDefaultController or ReadableByteStreamController and takes over + // all interaction with them. // // The ReadableStreamDefaultController can be used only so long as the JavaScript // code only enqueues ArrayBufferView or ArrayBuffer values. Everything else will @@ -1362,66 +971,29 @@ class ReadableStreamJsSource: public kj::Refcounted, // controller returns a value that cannot be intrepreted as bytes, then the source errors // and the read promise is rejected. // - // The source maintains an internal byte buffer. If the current read can be minimally - // fulfilled (minBytes) from the buffer, then it is and the read promise is resolved - // synchronously. Otherwise the source will read from the controller. If that returns - // enough data to fulfill the read request, then we're done. Whatever extra data it - // returns is stored in the buffer for the next read. If it does not return enough data, - // we'll keep pulling from the controller until it does or until the controller closes. + // It is possible for the underlying source to return more bytes than the current read can + // handle. To account for this case, the source maintains an internal byte buffer of its own. + // If the current read can be minimally fulfilled (minBytes) from that buffer, then it is and + // the read promise is resolved synchronously. Otherwise the source will read from the + // controller. If that returns enough data to fulfill the read request, then we're done. Whatever + // extra data it returns is stored in the buffer for the next read. If it does not return enough + // data, we'll keep pulling from the controller until it does or until the controller closes. public: - using ByobController = jscontroller::ByobController; - using DefaultController = jscontroller::DefaultController; - using Controller = kj::OneOf; - - explicit ReadableStreamJsSource(StreamStates::Closed closed) - : ioContext(IoContext::current()), - state(closed), - readPending(false), - canceling(false) {} - - explicit ReadableStreamJsSource(kj::Exception errored) - : ioContext(IoContext::current()), - state(kj::mv(errored)), - readPending(false), - canceling(false) {} - - explicit ReadableStreamJsSource(Controller controller) - : ioContext(IoContext::current()), - state(kj::mv(controller)), - readPending(false), - canceling(false) { - KJ_IF_MAYBE(controller, state.tryGet()) { - (*controller)->setOwner(*this); - } else KJ_IF_MAYBE(controller, state.tryGet()) { - (*controller)->setOwner(*this); - } else { - KJ_UNREACHABLE; - } - } - - ~ReadableStreamJsSource() noexcept(false) { - // This is defensive as detachFromController should have already been called. - // This will ensure if the controller is still attached, it's c++ reference - // to this source is cleared. - detachFromController(); - } - - void cancel(kj::Exception reason) override; + explicit ReadableStreamJsSource(StreamStates::Closed closed); + explicit ReadableStreamJsSource(kj::Exception errored); + explicit ReadableStreamJsSource(kj::Own consumer); + explicit ReadableStreamJsSource(kj::Own consumer); - void doClose() override; - - void doError(jsg::Lock& js, v8::Local reason) override; - - bool isLocked() const override; + void doClose(); + void doError(jsg::Lock& js, v8::Local reason); - bool isLockedReaderByteOriented() override; + // ReadableStreamSource implementation + void cancel(kj::Exception reason) override; kj::Promise tryRead(void* buffer, size_t minBytes, size_t maxBytes) override; - kj::Promise> pumpTo(WritableStreamSink& output, bool end) override; private: - void detachFromController(); jsg::Promise internalTryRead( jsg::Lock& js, void* buffer, @@ -1456,121 +1028,13 @@ class ReadableStreamJsSource: public kj::Refcounted, IoContext& ioContext; kj::OneOf state; + kj::Own, + kj::Own> state; std::deque queue; bool readPending = false; - bool canceling = false; }; -class ReadableStreamJsTeeSource: public kj::Refcounted, - public ReadableStreamSource, - public ReadableStreamController::TeeController::Branch { - // A ReadableStreamSource that sits on top of a ReadableStreamJSTeeAdapter. - // The layering here is fairly complicated. The tee adapter itself wraps - // either a ReadableStreamDefaultController or a ReadableByteStreamController. - // It is the job of the tee adapter to perform the actual pull/read from the underlying - // controller (which exists and operates in JavaScript heap space). Every time - // the tee adapter reads a chunk of data, it will push that chunk out to all - // of the attached branches. Initially, the attached branches are always - // ReadableStream's using the ReadableStreamJsTeeController. When the - // removeSource() method is called on the ReadableStreamJsTeeController, it - // gives it's reference to the tee adapter to the newly created - // ReadableStreamJsTeeSource. The new ReadableStreamJsTeeSource replaces the - // ReadableStreamJsTeeController as the branch that is registered with the tee adapter. - // The ReadableStreamJsTeeSource will then receive chunks of data from the - // tee adapter every time it performs a read on the underlying controller. - // - // The ReadableStreamJsTeeSource maintains an internal byte buffer. Whenever - // the tee adapter pushes data into the source and there is no currently - // pending read, the data is copied into that byte buffer. - // - // When tryRead is called, there are several steps: - // If the read can be fulfilled completely from the byte buffer, - // then it is and the read is synchronously fulfilled. - // - // Otherwise, the read is marked pending and the tee adapter is asked - // to pull more data. The promise will be fulfilled when the adapter - // delivers that data. - // - // If the adapter delivers more data than is necessary, the extra data - // is pushed into the buffer to be read later. If the adapter delivers - // less data than is necessary (minBytes), then the pendingRead is held - // and the tee adapter is asked to pull data again. It will keep pulling - // until the minimum number of bytes for the current read are provided. -public: - using TeeController = ReadableStreamController::TeeController; - using Readable = jsg::Ref; - - explicit ReadableStreamJsTeeSource( - StreamStates::Closed closed) - : ioContext(IoContext::current()), - state(closed) {} - - explicit ReadableStreamJsTeeSource(kj::Exception errored) - : ioContext(IoContext::current()), - state(kj::mv(errored)) {} - - explicit ReadableStreamJsTeeSource( - Readable readable, - TeeController& teeController, - std::deque bytes) - : ioContext(IoContext::current()), - state(kj::mv(readable)), - teeController(teeController), - queue(kj::mv(bytes)) { - KJ_ASSERT_NONNULL(this->teeController).addBranch(this); - } - - ~ReadableStreamJsTeeSource() noexcept(false) { - // There's a good chance that we're cleaning up here during garbage collection. - // In that case, we want to make sure we do not cancel any pending reads as that - // would involve allocating stuff during gc which is a no no. - detach(nullptr, nullptr); - } - - void cancel(kj::Exception reason) override; - - void detach(kj::Maybe maybeException, kj::Maybe maybeJs); - // See the comment for removeBranch in common.h for details on the use of maybeJs - - void doClose() override; - - void doError(jsg::Lock& js, v8::Local reason) override; - - void handleData(jsg::Lock& js, ReadResult result) override; - - kj::Promise tryRead(void* buffer, size_t minBytes, size_t maxBytes) override; - - kj::Promise> pumpTo(WritableStreamSink& output, bool end) override; - -private: - jsg::Promise internalTryRead( - jsg::Lock& js, - void* buffer, - size_t minBytes, - size_t maxBytes); - - jsg::Promise pipeLoop( - jsg::Lock& js, - WritableStreamSink& output, - bool end, - kj::Array bytes); - - IoContext& ioContext; - kj::OneOf state; - kj::Maybe teeController; - std::deque queue; - - struct PendingRead { - jsg::Promise::Resolver resolver; - kj::ArrayPtr bytes; - size_t minBytes; - size_t filled; - }; - - kj::Maybe pendingRead; -}; +// ======================================================================================= class WritableStreamDefaultController: public jsg::Object { // The WritableStreamDefaultController is an object defined by the stream specification. @@ -1578,7 +1042,6 @@ class WritableStreamDefaultController: public jsg::Object { // to determine whether it is capable of handling whatever type of JavaScript object it // is given. public: - using QueueType = jscontroller::ValueQueue; using WritableImpl = jscontroller::WritableImpl; using WriterOwner = jscontroller::WriterOwner; diff --git a/src/workerd/jsg/buffersource.c++ b/src/workerd/jsg/buffersource.c++ index e729c0d9b82..35ca7ceac57 100644 --- a/src/workerd/jsg/buffersource.c++ +++ b/src/workerd/jsg/buffersource.c++ @@ -76,6 +76,12 @@ BackingStore::BackingStore( kj::str("byteLength must be a multiple of ", this->elementSize, ".")); } +bool BackingStore::operator==(const BackingStore& other) { + return backingStore == other.backingStore && + byteLength == other.byteLength && + byteOffset == other.byteOffset; +} + BufferSource::BufferSource(Lock& js, v8::Local handle) : handle(js.v8Ref(handle)), maybeBackingStore(BackingStore( diff --git a/src/workerd/jsg/buffersource.h b/src/workerd/jsg/buffersource.h index fa1bcfabafa..d9fd7368b73 100644 --- a/src/workerd/jsg/buffersource.h +++ b/src/workerd/jsg/buffersource.h @@ -118,6 +118,8 @@ class BackingStore { inline operator kj::ArrayPtr() KJ_LIFETIMEBOUND { return asArrayPtr(); } + bool operator==(const BackingStore& other); + inline const kj::ArrayPtr asArrayPtr() const KJ_LIFETIMEBOUND { KJ_ASSERT(backingStore != nullptr, "Invalid access after move."); return kj::ArrayPtr( @@ -150,6 +152,23 @@ class BackingStore { checkIsIntegerType()); } + template + BackingStore getTypedViewSlice(size_t start, size_t end) { + KJ_ASSERT(start <= end); + auto length = end - start; + auto startOffset = byteOffset + start; + KJ_ASSERT(length <= byteLength); + KJ_ASSERT(startOffset <= backingStore->ByteLength()); + KJ_ASSERT(startOffset + length <= backingStore->ByteLength()); + return BackingStore( + backingStore, + length, + startOffset, + getBufferSourceElementSize(), + construct, + checkIsIntegerType()); + } + inline v8::Local createHandle(Lock& js) { return ctor(js, *this); } From 720689023463faf4ca56ee78d0951803062cd219 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 10 Oct 2022 07:22:37 -0700 Subject: [PATCH 4/6] Revert "Revert "Fixup regression in BYOB reads"" This reverts commit ab8fb4dd39beade9d80be66661915c45208896b0. --- src/workerd/api/streams/queue.c++ | 193 ++++++++++++++++++++++++++++-- src/workerd/api/streams/queue.h | 24 +++- 2 files changed, 206 insertions(+), 11 deletions(-) diff --git a/src/workerd/api/streams/queue.c++ b/src/workerd/api/streams/queue.c++ index be99ec81f09..f4ededce772 100644 --- a/src/workerd/api/streams/queue.c++ +++ b/src/workerd/api/streams/queue.c++ @@ -188,6 +188,16 @@ void ValueQueue::handleRead( } } +bool ValueQueue::handleMaybeClose( + jsg::Lock&js, + ConsumerImpl::Ready& state, + ConsumerImpl& consumer, + QueueImpl& queue) { + // If the value queue is not yet empty we have to keep waiting for more reads to consume it. + // Return false to indicate that we cannot close yet. + return false; +} + size_t ValueQueue::getConsumerCount() { return impl.getConsumerCount(); } #pragma endregion ValueQueue @@ -208,11 +218,23 @@ void maybeInvalidateByobRequest(kj::Maybe& req) { } // namespace void ByteQueue::ReadRequest::resolveAsDone(jsg::Lock& js) { - pullInto.store.trim(pullInto.store.size() - pullInto.filled); - resolver.resolve(ReadResult { - .value = js.v8Ref(pullInto.store.createHandle(js)), - .done = true - }); + if (pullInto.filled > 0) { + // There's been at least some data written, we need to respond but not + // set done to true since that's what the streams spec requires. + pullInto.store.trim(pullInto.store.size() - pullInto.filled); + resolver.resolve(ReadResult { + .value = js.v8Ref(pullInto.store.createHandle(js)), + .done = false + }); + } else { + // Otherwise, we set the length to zero + pullInto.store.trim(pullInto.store.size()); + KJ_ASSERT(pullInto.store.size() == 0); + resolver.resolve(ReadResult { + .value = js.v8Ref(pullInto.store.createHandle(js)), + .done = true + }); + } maybeInvalidateByobRequest(byobReadRequest); } @@ -427,7 +449,9 @@ v8::Local ByteQueue::ByobRequest::getView(jsg::Lock& js) { ByteQueue::ByteQueue(size_t highWaterMark) : impl(highWaterMark) {} -void ByteQueue::close(jsg::Lock& js) { impl.close(js); } +void ByteQueue::close(jsg::Lock& js) { + impl.close(js); +} ssize_t ByteQueue::desiredSize() const { return impl.desiredSize(); } @@ -688,7 +712,6 @@ void ByteQueue::handleRead( if (state.queueTotalSize > 0 && consume(state.queueTotalSize)) { return request.resolveAsDone(js); } - return pendingRead(); } @@ -720,6 +743,162 @@ void ByteQueue::handleRead( } } +bool ByteQueue::handleMaybeClose( + jsg::Lock&js, + ConsumerImpl::Ready& state, + ConsumerImpl& consumer, + QueueImpl& queue) { + // This is called when we know that we are closing and we still have data in + // the queue. We want to see if we can drain as much of it into pending reads + // as possible. If we're able to drain all of it, then yay! We can go ahead and + // close. Otherwise we stay open and wait for more reads to consume the rest. + + // We should only be here if there is data remaining in the queue. + KJ_ASSERT(state.queueTotalSize > 0); + + // We should also only be here if the consumer is closing. + KJ_ASSERT(consumer.isClosing()); + + const auto consume = [&] { + // Consume will copy as much of the remaining data in the buffer as possible + // to the next pending read. If the remaining data can fit into the remaining + // space in the read, awesome, we've consumed everything and we will return + // true. If the remaining data cannot fit into the remaining space in the read, + // then we'll return false to indicate that there's more data to consume. In + // either case, the pending read is popped off the pending queue and resolved. + + KJ_ASSERT(!state.readRequests.empty()); + auto& pending = state.readRequests.front(); + + while (!state.buffer.empty()) { + auto& next = state.buffer.front(); + KJ_SWITCH_ONEOF(next) { + KJ_CASE_ONEOF(c, ConsumerImpl::Close) { + // We've reached the end! queueTotalSize should be zero. We need to + // resolve and pop the current read and return true to indicate that + // we're all done. + // + // Technically, we really shouldn't get here but the case is covered + // just in case. + KJ_ASSERT(state.queueTotalSize == 0); + pending.resolve(js); + state.readRequests.pop_front(); + return true; + } + KJ_CASE_ONEOF(entry, QueueEntry) { + auto sourcePtr = entry.entry->toArrayPtr(); + auto sourceSize = sourcePtr.size() - entry.offset; + + auto destPtr = pending.pullInto.store.asArrayPtr().begin() + pending.pullInto.filled; + auto destAmount = pending.pullInto.store.size() - pending.pullInto.filled; + + // There should be space available to copy into and data to copy from, or + // something else went wrong. + KJ_ASSERT(destAmount > 0); + KJ_ASSERT(sourceSize > 0); + + // sourceSize is the amount of data remaining in the current entry to copy. + // destAmount is the amount of space remaining to be filled in the pending read. + auto amountToCopy = kj::min(sourceSize, destAmount); + + auto sourceStart = sourcePtr.begin() + entry.offset; + auto sourceEnd = sourceStart + amountToCopy; + + // It shouldn't be possible for sourceEnd to extend past the sourcePtr.end() + // but let's make sure just to be safe. + KJ_ASSERT(sourceEnd <= sourcePtr.end()); + + // Safely copy amountToCopy bytes from the source into the destination. + std::copy(sourceStart, sourceEnd, destPtr); + + pending.pullInto.filled += amountToCopy; + state.queueTotalSize -= amountToCopy; + entry.offset += amountToCopy; + + KJ_ASSERT(entry.offset <= sourcePtr.size()); + + if (sourceEnd == sourcePtr.end()) { + // If sourceEnd is equal to sourcePtr.end(), we've consumed the entire entry + // and we can free it. + auto released = kj::mv(next); + state.buffer.pop_front(); + + if (amountToCopy == destAmount) { + // If the amountToCopy is equal to destAmount, then we've completely filled + // this read request with the data remaining. Resolve the read request. If + // state.queueTotalSize happens to be zero, we can safely indicate that we + // have read the remaining data as this may have been the last actual value + // entry in the buffer. + pending.resolve(js); + state.readRequests.pop_front(); + + if (state.queueTotalSize == 0) { + // If the queueTotalSize is zero at this point, the next item in the queue + // must be a close and we can return true. All of the data has been consumed. + KJ_ASSERT(state.buffer.front().is()); + return true; + } + + // Otherwise, there's still data to consume, return false here to move on + // to the next pending read (if any). + return false; + } + + // We know that amountToCopy cannot be greater than destAmount because + // of the kj::min above. + + // Continuing here means that our pending read still has space to fill + // and we might still have value entries to fill it. We'll iterate around + // and see where we get. + continue; + } + + // This read did not consume everything in this entry but doesn't have + // any more space to fill. We will resolve this read and return false + // to indicate that the outer loop should continue with the next read + // request if there is one. + + // At this point, it should be impossible for state.queueTotalSize to + // be zero because there is still data remaining to be consumed in this + // buffer. + KJ_ASSERT(state.queueTotalSize > 0); + + pending.resolve(js); + state.readRequests.pop_front(); + return false; + } + } + KJ_UNREACHABLE; + } + + return state.queueTotalSize == 0; + }; + + // We can only consume here if there are pending reads! + while (!state.readRequests.empty()) { + // We ignore the read request atLeast here since we are closing. Our goal is to + // consume as much of the data as possible. + + if (consume()) { + // If consume returns true, we reached the end and have no more data to + // consume. That's a good thing! It means we can go ahead and close down. + return true; + } + + // If consume() returns false, there is still data left to consume in the queue. + // We will loop around and try again so long as there are still read requests + // pending. + } + + // At this point, we shouldn't have any read requests and there should be data + // left in the queue. We have to keep waiting for more reads to consume the + // remaining data. + KJ_ASSERT(state.queueTotalSize > 0); + KJ_ASSERT(state.readRequests.empty()); + + return false; +} + kj::Maybe> ByteQueue::nextPendingByobReadRequest() { KJ_IF_MAYBE(state, impl.getState()) { while (!state->pendingByobReadRequests.empty()) { diff --git a/src/workerd/api/streams/queue.h b/src/workerd/api/streams/queue.h index 2aafa7c903f..5d06a8949e0 100644 --- a/src/workerd/api/streams/queue.h +++ b/src/workerd/api/streams/queue.h @@ -549,10 +549,18 @@ class ConsumerImpl final { // released it. } } else { - // Otherwise, if the buffer is empty isClosing() is true, resolve the - // remaining read promises with close indicators and update the state - // to closed. If the buffer is not empty, do nothing. - if (empty() && isClosing()) { + // Otherwise, if isClosing() is true... + if (isClosing()) { + if (!empty() && !Self::handleMaybeClose(js, *ready, *this, queue)) { + // If the queue is not empty, we'll have the implementation see + // if it can drain the remaining data into pending reads. If handleMaybeClose + // returns false, then it could not and we can't yet close. If it returns true, + // yay! Our queue is empty and we can continue closing down. + KJ_ASSERT(!empty()); // We're still not empty + return; + } + + KJ_ASSERT(empty()); KJ_REQUIRE(ready->buffer.size() == 1); // The close should be the only item remaining. for (auto& request : ready->readRequests) { request.resolveAsDone(js); @@ -698,6 +706,10 @@ class ValueQueue final { ConsumerImpl& consumer, QueueImpl& queue, ReadRequest request); + static bool handleMaybeClose(jsg::Lock& js, + ConsumerImpl::Ready& state, + ConsumerImpl& consumer, + QueueImpl& queue); friend ConsumerImpl; }; @@ -892,6 +904,10 @@ class ByteQueue final { ConsumerImpl& consumer, QueueImpl& queue, ReadRequest request); + static bool handleMaybeClose(jsg::Lock& js, + ConsumerImpl::Ready& state, + ConsumerImpl& consumer, + QueueImpl& queue); friend ConsumerImpl; friend class Consumer; From a743f4c73d70335520e410601e60c0b14e65826a Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 8 Oct 2022 13:25:37 +0100 Subject: [PATCH 5/6] Fixup streams/queue.c++ for tee backpressure bug Two bugs here being fixed. --- src/workerd/api/streams/queue.c++ | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/workerd/api/streams/queue.c++ b/src/workerd/api/streams/queue.c++ index f4ededce772..b2b960f5e02 100644 --- a/src/workerd/api/streams/queue.c++ +++ b/src/workerd/api/streams/queue.c++ @@ -642,8 +642,12 @@ void ByteQueue::handleRead( // Because ReadRequest is movable, and because the ByobRequest captures // a reference to the ReadRequest, we wait until after it is added to // state.readRequests to create the associated ByobRequest. - KJ_REQUIRE_NONNULL(queue.getState()).pendingByobReadRequests.push_back( - state.readRequests.back().makeByobReadRequest(consumer, queue)); + // If the queue state is nullptr here, it means the queue has already + // been closed. + KJ_IF_MAYBE(queueState, queue.getState()) { + queueState->pendingByobReadRequests.push_back( + state.readRequests.back().makeByobReadRequest(consumer, queue)); + } } KJ_IF_MAYBE(listener, consumer.stateListener) { listener->onConsumerWantsData(js); @@ -868,7 +872,6 @@ bool ByteQueue::handleMaybeClose( return false; } } - KJ_UNREACHABLE; } return state.queueTotalSize == 0; From 10818d5d7da76feccba5ab5bb878adfc37c70e1a Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 10 Oct 2022 07:45:12 -0700 Subject: [PATCH 6/6] Fixup read into buffer size and offset calculation --- src/workerd/api/streams/internal.c++ | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/workerd/api/streams/internal.c++ b/src/workerd/api/streams/internal.c++ index 7713e805e7e..0752a7c2642 100644 --- a/src/workerd/api/streams/internal.c++ +++ b/src/workerd/api/streams/internal.c++ @@ -403,7 +403,10 @@ kj::Maybe> ReadableStreamInternalController::read( store = v8::ArrayBuffer::NewBackingStore(js.v8Isolate, byteLength); } - auto bytes = kj::arrayPtr(static_cast(store->Data()), byteOffset + byteLength); + KJ_ASSERT(store->ByteLength() == byteOffset + byteLength); + + auto ptr = static_cast(store->Data()); + auto bytes = kj::arrayPtr(ptr + byteOffset, byteLength); disturbed = true; KJ_SWITCH_ONEOF(state) {