MOZGIII / xwt

A unified WebTransport API that you can use for browser/native cross platform apps. Originally to power the networking layer of the games made with the bevy engine - but can be applied elsewhere too.
31 stars 4 forks source link

Changing buffer size when reading from AsyncRead doesn't work #111

Closed vpzomtrrfrt closed 5 months ago

vpzomtrrfrt commented 6 months ago

For example, by changing this test:

diff --git a/crates/xwt-tests/src/tests/tokio_io_read_small_buf.rs b/crates/xwt-tests/src/tests/tokio_io_read_small_buf.rs
index 889d318..6b0ad1b 100644
--- a/crates/xwt-tests/src/tests/tokio_io_read_small_buf.rs
+++ b/crates/xwt-tests/src/tests/tokio_io_read_small_buf.rs
@@ -39,7 +39,7 @@ where

     tokio::pin!(send_stream);

-    let mut to_write = &b"hello"[..];
+    let mut to_write = &b"hello!"[..];
     loop {
         let written = tokio::io::AsyncWriteExt::write(&mut send_stream, to_write)
             .await
@@ -66,16 +66,18 @@ where
         return Err(Error::BadData(read_buf));
     }

-    let read = tokio::io::AsyncReadExt::read(&mut recv_stream, &mut read_buf[..])
+    let mut read_buf_2 = vec![0u8; 3];
+
+    let read = tokio::io::AsyncReadExt::read(&mut recv_stream, &mut read_buf_2[..])
         .await
         .map_err(Error::Read)?;
     if read == 0 {
         return Err(Error::NoResponse);
     };
-    read_buf.truncate(read);
+    read_buf_2.truncate(read);

-    if read_buf != b"ll" {
-        return Err(Error::BadData(read_buf));
+    if read_buf_2 != b"llo" {
+        return Err(Error::BadData(read_buf_2));
     }

     let read = tokio::io::AsyncReadExt::read(&mut recv_stream, &mut read_buf[..])
@@ -86,7 +88,7 @@ where
     };
     read_buf.truncate(read);

-    if read_buf != b"o" {
+    if read_buf != b"!" {
         return Err(Error::BadData(read_buf));
     }

It now fails with:

panicked at crates/xwt-web-sys/tests/test.rs:97:10:
called `Result::unwrap()` on an `Err` value: BadData([108, 108])

Which seems to imply the internal buffer was not changed.

Making the second buffer smaller also fails, with:

panicked at crates/web-sys-async-io/src/reader.rs:64:39:
n overflows remaining
MOZGIII commented 5 months ago

Good point, it is indeed an issue with the implementation; I initially did this intentionally as I did not expect anyone to have a use case for this - as this would require reallocating buffer, which would be more efficient if avoided. It is fairly easy to correct, and indeed should be either corrected or at least documented.

Can you provide more info on how you are using xwt that you need to change the buffer size dynamically?

vpzomtrrfrt commented 5 months ago

I'm using async-bincode, which does that internally

MOZGIII commented 5 months ago

@vpzomtrrfrt could you please try the code at #124? It should fix the issue.