diff --git a/CHANGELOG.md b/CHANGELOG.md index abc0a11..92846a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## Unreleased + +* Updated documentation of `ReadableStream(Default|BYOB)Reader::release_lock()` around the expected behavior when there are pending read requests. + See the corresponding [Streams specification change](https://github.com/whatwg/streams/commit/d5f92d9f17306d31ba6b27424d23d58e89bf64a5) for details. + ## v0.2.3 (2022-05-18) * Replaced `futures` dependency with `futures-util` to improve compilation times ([#11](https://github.com/MattiasBuelens/wasm-streams/pull/11), [#12](https://github.com/MattiasBuelens/wasm-streams/pull/12)) diff --git a/src/readable/byob_reader.rs b/src/readable/byob_reader.rs index 2130359..61490f1 100644 --- a/src/readable/byob_reader.rs +++ b/src/readable/byob_reader.rs @@ -14,9 +14,6 @@ use super::{sys, IntoAsyncRead, ReadableStream}; /// This is returned by the [`get_byob_reader`](ReadableStream::get_byob_reader) method. /// /// When the reader is dropped, it automatically [releases its lock](https://streams.spec.whatwg.org/#release-a-lock). -/// If the reader still has a pending read request at this point (i.e. if a future returned -/// by [`read`](Self::read) is not yet ready), then this will **panic**. You must either `await` -/// all `read` futures, or [`cancel`](Self::cancel) the stream to discard any pending `read` futures. #[derive(Debug)] pub struct ReadableStreamBYOBReader<'stream> { raw: sys::ReadableStreamBYOBReader, @@ -146,9 +143,14 @@ impl<'stream> ReadableStreamBYOBReader<'stream> { /// [Releases](https://streams.spec.whatwg.org/#release-a-lock) this reader's lock on the /// corresponding stream. /// - /// **Panics** if the reader still has a pending read request, i.e. if a future returned - /// by [`read`](Self::read) is not yet ready. For a non-panicking variant, - /// use [`try_release_lock`](Self::try_release_lock). + /// [As of January 2022](https://github.com/whatwg/streams/commit/d5f92d9f17306d31ba6b27424d23d58e89bf64a5), + /// the Streams standard allows the lock to be released even when there are still pending read + /// requests. Such requests will automatically become rejected, and this function will always + /// succeed. + /// + /// However, if the Streams implementation is not yet up-to-date with this change, then + /// releasing the lock while there are pending read requests will **panic**. For a non-panicking + /// variant, use [`try_release_lock`](Self::try_release_lock). #[inline] pub fn release_lock(mut self) { self.release_lock_mut() @@ -163,8 +165,13 @@ impl<'stream> ReadableStreamBYOBReader<'stream> { /// Try to [release](https://streams.spec.whatwg.org/#release-a-lock) this reader's lock on the /// corresponding stream. /// - /// The lock cannot be released while the reader still has a pending read request, i.e. - /// if a future returned by [`read`](Self::read) is not yet ready. Attempting to do so will + /// [As of January 2022](https://github.com/whatwg/streams/commit/d5f92d9f17306d31ba6b27424d23d58e89bf64a5), + /// the Streams standard allows the lock to be released even when there are still pending read + /// requests. Such requests will automatically become rejected, and this function will always + /// return `Ok(())`. + /// + /// However, if the Streams implementation is not yet up-to-date with this change, then + /// the lock cannot be released while there are pending read requests. Attempting to do so will /// return an error and leave the reader locked to the stream. #[inline] pub fn try_release_lock(self) -> Result<(), (js_sys::Error, Self)> { diff --git a/src/readable/default_reader.rs b/src/readable/default_reader.rs index 2ec6424..c522abc 100644 --- a/src/readable/default_reader.rs +++ b/src/readable/default_reader.rs @@ -13,9 +13,6 @@ use super::{sys, IntoStream, ReadableStream}; /// This is returned by the [`get_reader`](ReadableStream::get_reader) method. /// /// When the reader is dropped, it automatically [releases its lock](https://streams.spec.whatwg.org/#release-a-lock). -/// If the reader still has a pending read request at this point (i.e. if a future returned -/// by [`read`](Self::read) is not yet ready), then this will **panic**. You must either `await` -/// all `read` futures, or [`cancel`](Self::cancel) the stream to discard any pending `read` futures. #[derive(Debug)] pub struct ReadableStreamDefaultReader<'stream> { raw: sys::ReadableStreamDefaultReader, @@ -80,9 +77,14 @@ impl<'stream> ReadableStreamDefaultReader<'stream> { /// [Releases](https://streams.spec.whatwg.org/#release-a-lock) this reader's lock on the /// corresponding stream. /// - /// **Panics** if the reader still has a pending read request, i.e. if a future returned - /// by [`read`](Self::read) is not yet ready. For a non-panicking variant, - /// use [`try_release_lock`](Self::try_release_lock). + /// [As of January 2022](https://github.com/whatwg/streams/commit/d5f92d9f17306d31ba6b27424d23d58e89bf64a5), + /// the Streams standard allows the lock to be released even when there are still pending read + /// requests. Such requests will automatically become rejected, and this function will always + /// succeed. + /// + /// However, if the Streams implementation is not yet up-to-date with this change, then + /// releasing the lock while there are pending read requests will **panic**. For a non-panicking + /// variant, use [`try_release_lock`](Self::try_release_lock). #[inline] pub fn release_lock(mut self) { self.release_lock_mut() @@ -97,8 +99,13 @@ impl<'stream> ReadableStreamDefaultReader<'stream> { /// Try to [release](https://streams.spec.whatwg.org/#release-a-lock) this reader's lock on the /// corresponding stream. /// - /// The lock cannot be released while the reader still has a pending read request, i.e. - /// if a future returned by [`read`](Self::read) is not yet ready. Attempting to do so will + /// [As of January 2022](https://github.com/whatwg/streams/commit/d5f92d9f17306d31ba6b27424d23d58e89bf64a5), + /// the Streams standard allows the lock to be released even when there are still pending read + /// requests. Such requests will automatically become rejected, and this function will always + /// return `Ok(())`. + /// + /// However, if the Streams implementation is not yet up-to-date with this change, then + /// the lock cannot be released while there are pending read requests. Attempting to do so will /// return an error and leave the reader locked to the stream. #[inline] pub fn try_release_lock(self) -> Result<(), (js_sys::Error, Self)> { diff --git a/src/readable/into_async_read.rs b/src/readable/into_async_read.rs index 4333a65..876efd7 100644 --- a/src/readable/into_async_read.rs +++ b/src/readable/into_async_read.rs @@ -20,13 +20,6 @@ use super::ReadableStreamBYOBReader; /// When this `AsyncRead` is dropped, it also drops its reader which in turn /// [releases its lock](https://streams.spec.whatwg.org/#release-a-lock). /// -/// When used through [`ReadableStream::into_async_read`](super::ReadableStream::into_async_read), -/// the stream is automatically cancelled before dropping the reader, discarding any pending read requests. -/// When used through [`ReadableStreamBYOBReader::into_async_read`](super::ReadableStreamBYOBReader::into_async_read), -/// it is up to the user to either manually [cancel](Self::cancel) the stream, -/// or to ensure that there are no pending read requests when dropped. -/// See the documentation on [`ReadableStreamBYOBReader`] for more details on the drop behavior. -/// /// [`AsyncRead`]: https://docs.rs/futures/0.3.18/futures/io/trait.AsyncRead.html #[must_use = "readers do nothing unless polled"] #[derive(Debug)] diff --git a/src/readable/into_stream.rs b/src/readable/into_stream.rs index 3570c64..92f57d2 100644 --- a/src/readable/into_stream.rs +++ b/src/readable/into_stream.rs @@ -16,13 +16,6 @@ use super::ReadableStreamDefaultReader; /// When this `Stream` is dropped, it also drops its reader which in turn /// [releases its lock](https://streams.spec.whatwg.org/#release-a-lock). /// -/// When used through [`ReadableStream::into_stream`](super::ReadableStream::into_stream), -/// the stream is automatically cancelled before dropping the reader, discarding any pending read requests. -/// When used through [`ReadableStreamDefaultReader::into_stream`](super::ReadableStreamDefaultReader::into_stream), -/// it is up to the user to either manually [cancel](Self::cancel) the stream, -/// or to ensure that there are no pending read requests when dropped. -/// See the documentation on [`ReadableStreamDefaultReader`] for more details on the drop behavior. -/// /// [`Stream`]: https://docs.rs/futures/0.3.18/futures/stream/trait.Stream.html #[must_use = "streams do nothing unless polled"] #[derive(Debug)] diff --git a/tests/js/readable_stream.js b/tests/js/readable_stream.js index 06dcc54..8927022 100644 --- a/tests/js/readable_stream.js +++ b/tests/js/readable_stream.js @@ -45,4 +45,20 @@ export function new_readable_byte_stream_from_array(chunks) { } } }); -} \ No newline at end of file +} + +/** + * Tests whether `reader.releaseLock()` is allowed while there are pending read requests. + * + * See: https://github.com/whatwg/streams/commit/d5f92d9f17306d31ba6b27424d23d58e89bf64a5 + */ +export function supports_release_lock_with_pending_read() { + try { + const reader = new ReadableStream().getReader(); + reader.read().then(() => {}, () => {}); + reader.releaseLock(); + return true; + } catch { + return false; + } +} diff --git a/tests/js/readable_stream.rs b/tests/js/readable_stream.rs index 6027795..42f5059 100644 --- a/tests/js/readable_stream.rs +++ b/tests/js/readable_stream.rs @@ -8,4 +8,5 @@ extern "C" { pub fn new_noop_readable_byte_stream() -> sys::ReadableStream; pub fn new_readable_stream_from_array(chunks: Box<[JsValue]>) -> sys::ReadableStream; pub fn new_readable_byte_stream_from_array(chunks: Box<[JsValue]>) -> sys::ReadableStream; + pub fn supports_release_lock_with_pending_read() -> bool; } diff --git a/tests/tests/readable_byte_stream.rs b/tests/tests/readable_byte_stream.rs index 282a7ca..a7de7ad 100644 --- a/tests/tests/readable_byte_stream.rs +++ b/tests/tests/readable_byte_stream.rs @@ -205,6 +205,33 @@ async fn test_readable_byte_stream_multiple_byob_readers() { } async fn test_readable_byte_stream_abort_read(mut readable: ReadableStream) { + if supports_release_lock_with_pending_read() { + test_readable_byte_stream_abort_read_new(readable).await; + } else { + test_readable_byte_stream_abort_read_old(readable).await; + } +} + +async fn test_readable_byte_stream_abort_read_new(mut readable: ReadableStream) { + let mut reader = readable.get_byob_reader(); + + // Start reading + // Since the stream will never produce a chunk, this read will remain pending forever + let mut dst = [0u8; 3]; + let mut fut = reader.read(&mut dst).boxed_local(); + // We need to poll the future at least once to start the read + let poll_result = poll!(&mut fut); + assert!(matches!(poll_result, Poll::Pending)); + // Drop the future, to regain control over the reader + drop(fut); + + // Releasing the lock should work even while there are pending reads + reader + .try_release_lock() + .expect("releasing the reader should work even while there are pending reads"); +} + +async fn test_readable_byte_stream_abort_read_old(mut readable: ReadableStream) { let mut reader = readable.get_byob_reader(); // Start reading diff --git a/tests/tests/readable_stream.rs b/tests/tests/readable_stream.rs index 00e8c47..7ba6f5c 100644 --- a/tests/tests/readable_stream.rs +++ b/tests/tests/readable_stream.rs @@ -123,6 +123,35 @@ async fn test_readable_stream_multiple_readers() { #[wasm_bindgen_test] async fn test_readable_stream_abort_read() { + if supports_release_lock_with_pending_read() { + test_readable_stream_abort_read_new().await; + } else { + test_readable_stream_abort_read_old().await; + } +} + +async fn test_readable_stream_abort_read_new() { + let stream = pending(); + let (stream, observer) = observe_drop(stream); + let mut readable = ReadableStream::from_stream(stream); + let mut reader = readable.get_reader(); + + // Start reading + // Since the stream will never produce a chunk, this read will remain pending forever + let mut fut = reader.read().boxed_local(); + // We need to poll the future at least once to start the read + let poll_result = poll!(&mut fut); + assert!(matches!(poll_result, Poll::Pending)); + // Drop the future, to regain control over the reader + drop(fut); + + // Releasing the lock should work even while there are pending reads + reader + .try_release_lock() + .expect("releasing the reader should work even while there are pending reads"); +} + +async fn test_readable_stream_abort_read_old() { let stream = pending(); let (stream, observer) = observe_drop(stream); let mut readable = ReadableStream::from_stream(stream);