Skip to content

Should we "lock" readable streams while piping? #241

@domenic

Description

@domenic

Inspired by:

Right now, if you are piping a readable stream to a writable stream, you can also concurrently call .read(). Usually this will just cause a mess, but it fell out of the design naturally.

Both "off-main thread piping" (#97) and a similar situation with Fetch's Request class, which has a .json() method, deal with the hazards of "C++ readers" concurrent with potential "JS readers." Ideally once a stream is being read by C++, JS cannot touch it, both for performance and implementation-complexity reasons.

One solution for this might be to add some kind of "locking" mechanism so that only one consumer can read. But I don't know how to define that, without some kind of radical redesign that splits out the concept of readable stream into a stream and a stream reader or something like that.

Another possibility that would work quite well I think would be to lock the stream while it's being piped somewhere. That is, at the beginning of pipeTo, set a [[piping]] = true property; when pipeTo finishes, set [[piping]] = false. While [[piping]] is true, .read() instantly throws, saying that the stream is being piped elsewhere and cannot be read.

This would solve all of our problems that inspire this issue:

  • It removes the potential author footgun of accidentally calling .read() and getting racy or unexpected results, and perturbing the pipe in progress, that @acolwell was worried about.
  • Off-main-thread piping (of UA-created streams) would be able to react the same way, so that to author code it looks like any normal pipe.
  • Via some sleight of hand, it solves .json(), by "explaining" .json() as piping to a concatenator stream. (See Backpressure on fetch integrated with Streams w3c/ServiceWorker#452 (comment)) Under the hood the UA will probably not implement .json() that way, but what's important is that they could, with the same author-observable behavior---so that way, .json() isn't doing anything magic.

It has a few theoretical disadvantages, but I am not sure how big of a deal they are in practice:

  • It means ReadableStream.prototype.pipeTo will no longer be implementable purely on top of the public readable interface; it will be using some private [[piping]] state, and some private version of the .read() method that doesn't throw while the stream is [[piping]]. This means e.g. it doesn't apply directly to ReadableByteStreams. (Note however that it still only uses public writable stream interfaces, unless we want to lock those too...)
  • It removes the ability to pipe to multiple streams in order to have the one that is most free to accept writes pick them up. This sounds rare, but funnily enough just today I was asked about a npm package to do this. It could be built on top as a library though, similar to a tee stream but without any copies.

Note that this does not disable the use case of e.g. .read()ing headers from a stream and then piping the rest of it, or piping with { preventCancel: false } and .read()ing the leftover if the destination gets closed prematurely. It only disables concurrent pipes + reads, not sequential.

Would love thoughts from @tyoshino @yutakahirano @sicking and anyone else.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions