From 7c2cd168766fee2053e01d979a6f8a43a056d362 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 15 Jun 2025 06:45:19 +0100 Subject: [PATCH 1/5] Update decoding bounds for BorrowDecode --- src/message.rs | 10 +++++----- src/preamble.rs | 10 +++++----- src/server.rs | 28 +++++++++++++++++----------- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/message.rs b/src/message.rs index dc3acdc6..22afe290 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1,13 +1,13 @@ use bincode::error::{DecodeError, EncodeError}; -use bincode::{Decode, Encode, config, decode_from_slice, encode_to_vec}; +use bincode::{BorrowDecode, Encode, borrow_decode_from_slice, config, encode_to_vec}; /// Wrapper trait for application message types. /// -/// Any type deriving [`Encode`] and [`Decode`] automatically implements +/// Any type deriving [`Encode`] and [`BorrowDecode`] automatically implements /// this trait via a blanket implementation. The default methods provide /// convenient helpers to serialize and deserialize using bincode's /// standard configuration. -pub trait Message: Encode + Decode<()> { +pub trait Message: Encode + for<'de> BorrowDecode<'de, ()> { /// Serialize the message into a byte vector. /// /// # Errors @@ -27,8 +27,8 @@ pub trait Message: Encode + Decode<()> { where Self: Sized, { - decode_from_slice(bytes, config::standard()) + borrow_decode_from_slice(bytes, config::standard()) } } -impl Message for T where T: Encode + Decode<()> {} +impl Message for T where for<'de> T: Encode + BorrowDecode<'de, ()> {} diff --git a/src/preamble.rs b/src/preamble.rs index 81c9bc0c..67930652 100644 --- a/src/preamble.rs +++ b/src/preamble.rs @@ -1,5 +1,5 @@ use bincode::error::DecodeError; -use bincode::{Decode, config, decode_from_slice}; +use bincode::{BorrowDecode, borrow_decode_from_slice, config}; use tokio::io::{self, AsyncRead, AsyncReadExt}; const MAX_PREAMBLE_LEN: usize = 1024; @@ -55,9 +55,9 @@ where pub async fn read_preamble(reader: &mut R) -> Result<(T, Vec), DecodeError> where R: AsyncRead + Unpin, - // `Decode` expects a decoding context type, not a lifetime. Most callers - // use the unit type as the context, which requires no external state. - T: Decode<()>, + // `BorrowDecode` allows decoding types with borrowed data using any + // lifetime. We require no external context, hence `()`. + for<'de> T: BorrowDecode<'de, ()>, { let mut buf = Vec::new(); // Build the decoder configuration once to avoid repeated allocations. @@ -65,7 +65,7 @@ where .with_big_endian() .with_fixed_int_encoding(); loop { - match decode_from_slice::(&buf, config) { + match borrow_decode_from_slice::(&buf, config) { Ok((value, consumed)) => { let leftover = buf.split_off(consumed); return Ok((value, leftover)); diff --git a/src/server.rs b/src/server.rs index 98c195f8..ad9c574b 100644 --- a/src/server.rs +++ b/src/server.rs @@ -25,9 +25,10 @@ use crate::app::WireframeApp; pub struct WireframeServer where F: Fn() -> WireframeApp + Send + Sync + Clone + 'static, - // `Decode`'s type parameter represents a decoding context. - // The unit type signals that no context is required. - T: bincode::Decode<()> + Send + 'static, + // `Decode` accepts a lifetime parameter. We allow any borrow duration + // using a higher-ranked trait bound. The unit type indicates no external + // decoding context is required. + for<'de> T: bincode::BorrowDecode<'de, ()> + Send + 'static, { factory: F, listener: Option>, @@ -93,8 +94,10 @@ where #[must_use] pub fn with_preamble(self) -> WireframeServer where - // Unit context indicates no external state is required when decoding. - T: bincode::Decode<()> + Send + 'static, + // The decoding context is `()`. We permit any borrow duration using a + // higher-ranked trait bound so callers may decode types containing + // references. + for<'de> T: bincode::BorrowDecode<'de, ()> + Send + 'static, { WireframeServer { factory: self.factory, @@ -110,8 +113,9 @@ where impl WireframeServer where F: Fn() -> WireframeApp + Send + Sync + Clone + 'static, - // `Decode` is generic over a context type; we use `()` here. - T: bincode::Decode<()> + Send + 'static, + // Accept any borrow lifetime via `BorrowDecode`. No external context is + // required so we use `()`. + for<'de> T: bincode::BorrowDecode<'de, ()> + Send + 'static, { /// Set the number of worker tasks to spawn for the server. /// @@ -328,8 +332,9 @@ async fn worker_task( shutdown_rx: &mut broadcast::Receiver<()>, ) where F: Fn() -> WireframeApp + Send + Sync + Clone + 'static, - // The unit context indicates no additional state is needed to decode `T`. - T: bincode::Decode<()> + Send + 'static, + // Allow any lifetime for decoding borrowed preamble data via + // `BorrowDecode`. No external context is required. + for<'de> T: bincode::BorrowDecode<'de, ()> + Send + 'static, { let mut delay = Duration::from_millis(10); loop { @@ -361,8 +366,9 @@ async fn process_stream( on_failure: Option>, ) where F: Fn() -> WireframeApp + Send + Sync + 'static, - // The decoding context parameter is `()`; no external state is needed. - T: bincode::Decode<()> + Send + 'static, + // Decoding may borrow from the preamble; allow any lifetime via + // `BorrowDecode`. No external context is needed so we use `()`. + for<'de> T: bincode::BorrowDecode<'de, ()> + Send + 'static, { match read_preamble::<_, T>(&mut stream).await { Ok((preamble, leftover)) => { From 811634835aa7cf18b6b6510f7e0c3de61e8d45ff Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 15 Jun 2025 09:55:17 +0100 Subject: [PATCH 2/5] Simplify bincode bounds and preamble read --- docs/preamble-validator.md | 2 +- src/message.rs | 2 +- src/preamble.rs | 6 ++++-- src/server.rs | 35 ++++++++++++++++++----------------- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/docs/preamble-validator.md b/docs/preamble-validator.md index f0905fd2..2bdce726 100644 --- a/docs/preamble-validator.md +++ b/docs/preamble-validator.md @@ -4,7 +4,7 @@ client connects. The server decodes the preamble with [`read_preamble`](../src/preamble.rs) and can invoke user-supplied callbacks on success or failure. The helper uses `bincode` to decode any type implementing -`bincode::Decode` and reads exactly the number of bytes required. +`bincode::BorrowDecode` and reads exactly the number of bytes required. The flow is summarized below: diff --git a/src/message.rs b/src/message.rs index 22afe290..5efdd07a 100644 --- a/src/message.rs +++ b/src/message.rs @@ -31,4 +31,4 @@ pub trait Message: Encode + for<'de> BorrowDecode<'de, ()> { } } -impl Message for T where for<'de> T: Encode + BorrowDecode<'de, ()> {} +impl Message for T where T: Encode + for<'de> BorrowDecode<'de, ()> {} diff --git a/src/preamble.rs b/src/preamble.rs index 67930652..9cb3b8a6 100644 --- a/src/preamble.rs +++ b/src/preamble.rs @@ -55,11 +55,13 @@ where pub async fn read_preamble(reader: &mut R) -> Result<(T, Vec), DecodeError> where R: AsyncRead + Unpin, - // `BorrowDecode` allows decoding types with borrowed data using any - // lifetime. We require no external context, hence `()`. + // Decode borrowed data for any lifetime without external context. for<'de> T: BorrowDecode<'de, ()>, { + // Read a small chunk upfront to avoid a guaranteed decode failure on the + // first iteration. let mut buf = Vec::new(); + read_more(reader, &mut buf, 8.min(MAX_PREAMBLE_LEN)).await?; // Build the decoder configuration once to avoid repeated allocations. let config = config::standard() .with_big_endian() diff --git a/src/server.rs b/src/server.rs index ad9c574b..99f4754f 100644 --- a/src/server.rs +++ b/src/server.rs @@ -14,6 +14,13 @@ use bincode::error::DecodeError; use crate::app::WireframeApp; +/// Trait bound for preamble types accepted by the server. +/// +/// The bound allows decoding borrowed data for any lifetime without +/// requiring an external decoding context. +pub trait Preamble: for<'de> bincode::BorrowDecode<'de, ()> + Send + 'static {} +impl Preamble for T where for<'de> T: bincode::BorrowDecode<'de, ()> + Send + 'static {} + /// Tokio-based server for `WireframeApp` instances. /// /// `WireframeServer` spawns a worker task per thread. Each worker @@ -25,10 +32,9 @@ use crate::app::WireframeApp; pub struct WireframeServer where F: Fn() -> WireframeApp + Send + Sync + Clone + 'static, - // `Decode` accepts a lifetime parameter. We allow any borrow duration - // using a higher-ranked trait bound. The unit type indicates no external - // decoding context is required. - for<'de> T: bincode::BorrowDecode<'de, ()> + Send + 'static, + // `Preamble` covers types implementing `BorrowDecode` for any lifetime, + // enabling decoding of borrowed data without external context. + T: Preamble, { factory: F, listener: Option>, @@ -94,10 +100,8 @@ where #[must_use] pub fn with_preamble(self) -> WireframeServer where - // The decoding context is `()`. We permit any borrow duration using a - // higher-ranked trait bound so callers may decode types containing - // references. - for<'de> T: bincode::BorrowDecode<'de, ()> + Send + 'static, + // New preamble types must satisfy the `Preamble` bound. + T: Preamble, { WireframeServer { factory: self.factory, @@ -113,9 +117,8 @@ where impl WireframeServer where F: Fn() -> WireframeApp + Send + Sync + Clone + 'static, - // Accept any borrow lifetime via `BorrowDecode`. No external context is - // required so we use `()`. - for<'de> T: bincode::BorrowDecode<'de, ()> + Send + 'static, + // The preamble type must satisfy the `Preamble` bound. + T: Preamble, { /// Set the number of worker tasks to spawn for the server. /// @@ -332,9 +335,8 @@ async fn worker_task( shutdown_rx: &mut broadcast::Receiver<()>, ) where F: Fn() -> WireframeApp + Send + Sync + Clone + 'static, - // Allow any lifetime for decoding borrowed preamble data via - // `BorrowDecode`. No external context is required. - for<'de> T: bincode::BorrowDecode<'de, ()> + Send + 'static, + // `Preamble` ensures `T` supports borrowed decoding. + T: Preamble, { let mut delay = Duration::from_millis(10); loop { @@ -366,9 +368,8 @@ async fn process_stream( on_failure: Option>, ) where F: Fn() -> WireframeApp + Send + Sync + 'static, - // Decoding may borrow from the preamble; allow any lifetime via - // `BorrowDecode`. No external context is needed so we use `()`. - for<'de> T: bincode::BorrowDecode<'de, ()> + Send + 'static, + // `Preamble` ensures `T` supports borrowed decoding. + T: Preamble, { match read_preamble::<_, T>(&mut stream).await { Ok((preamble, leftover)) => { From 02142a659caaa713829cee51ccaec3474eaebf1b Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 15 Jun 2025 10:56:48 +0100 Subject: [PATCH 3/5] Move Preamble trait --- src/preamble.rs | 7 +++++++ src/server.rs | 13 +++---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/preamble.rs b/src/preamble.rs index 9cb3b8a6..937da433 100644 --- a/src/preamble.rs +++ b/src/preamble.rs @@ -4,6 +4,13 @@ use tokio::io::{self, AsyncRead, AsyncReadExt}; const MAX_PREAMBLE_LEN: usize = 1024; +/// Trait bound for types accepted as connection preambles. +/// +/// The bound allows decoding borrowed data for any lifetime without +/// requiring an external decoding context. +pub trait Preamble: for<'de> BorrowDecode<'de, ()> + Send + 'static {} +impl Preamble for T where for<'de> T: BorrowDecode<'de, ()> + Send + 'static {} + async fn read_more( reader: &mut R, buf: &mut Vec, diff --git a/src/server.rs b/src/server.rs index 99f4754f..fd696f91 100644 --- a/src/server.rs +++ b/src/server.rs @@ -8,19 +8,12 @@ use tokio::time::{Duration, sleep}; use core::marker::PhantomData; -use crate::preamble::read_preamble; +use crate::preamble::{Preamble, read_preamble}; use crate::rewind_stream::RewindStream; use bincode::error::DecodeError; use crate::app::WireframeApp; -/// Trait bound for preamble types accepted by the server. -/// -/// The bound allows decoding borrowed data for any lifetime without -/// requiring an external decoding context. -pub trait Preamble: for<'de> bincode::BorrowDecode<'de, ()> + Send + 'static {} -impl Preamble for T where for<'de> T: bincode::BorrowDecode<'de, ()> + Send + 'static {} - /// Tokio-based server for `WireframeApp` instances. /// /// `WireframeServer` spawns a worker task per thread. Each worker @@ -98,10 +91,10 @@ where /// Call this before registering preamble handlers, otherwise any /// previously configured callbacks will be dropped. #[must_use] - pub fn with_preamble(self) -> WireframeServer + pub fn with_preamble

(self) -> WireframeServer where // New preamble types must satisfy the `Preamble` bound. - T: Preamble, + P: Preamble, { WireframeServer { factory: self.factory, From b4d5ff53f80fa60336df4f4753f52c4deef24ea3 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 15 Jun 2025 11:16:45 +0100 Subject: [PATCH 4/5] Mark doctest examples as no_run --- src/extractor.rs | 4 ++-- src/server.rs | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/extractor.rs b/src/extractor.rs index 51ffec53..a20b9d6c 100644 --- a/src/extractor.rs +++ b/src/extractor.rs @@ -73,7 +73,7 @@ impl SharedState { /// /// # Examples /// - /// ``` + /// ```no_run /// use std::sync::Arc; /// use wireframe::extractor::SharedState; /// @@ -108,7 +108,7 @@ impl std::ops::Deref for SharedState { /// /// # Examples /// - /// ``` + /// ```no_run /// use std::sync::Arc; /// use wireframe::extractor::SharedState; /// diff --git a/src/server.rs b/src/server.rs index fd696f91..06f62f24 100644 --- a/src/server.rs +++ b/src/server.rs @@ -90,6 +90,15 @@ where /// /// Call this before registering preamble handlers, otherwise any /// previously configured callbacks will be dropped. + /// + /// # Examples + /// + /// ```no_run + /// use wireframe::{app::WireframeApp, server::WireframeServer}; + /// + /// let factory = || WireframeApp::new().unwrap(); + /// let server = WireframeServer::new(factory).with_preamble::<()>(); + /// ``` #[must_use] pub fn with_preamble

(self) -> WireframeServer where From 01291b174b4e7b8715a16f59cbbeda7cddb51ddb Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Sun, 15 Jun 2025 11:34:48 +0100 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=93=9D=20Add=20docstrings=20to=20`cod?= =?UTF-8?q?ex/fix-bincode-decode-type-bounds`=20(#47)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docstrings generation was requested by @leynos. * https://github.com/leynos/wireframe/pull/44#issuecomment-2973517002 The following files were modified: * `src/message.rs` * `src/preamble.rs` * `src/server.rs` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Leynos --- src/message.rs | 13 ++++++++++++- src/preamble.rs | 31 ++++++++++++++++++++++++++++++- src/server.rs | 24 ++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/message.rs b/src/message.rs index 5efdd07a..c0933c33 100644 --- a/src/message.rs +++ b/src/message.rs @@ -22,7 +22,18 @@ pub trait Message: Encode + for<'de> BorrowDecode<'de, ()> { /// /// # Errors /// - /// Returns a [`DecodeError`] if deserialization fails. + /// Deserialises a message instance from a byte slice using the standard configuration. + /// + /// Returns the deserialised message and the number of bytes consumed, or a [`DecodeError`] if deserialisation fails. + /// + /// # Examples + /// + /// ``` + /// use your_crate::Message; + /// let bytes = /* some valid serialised message bytes */; + /// let (msg, consumed) = MyMessageType::from_bytes(&bytes).unwrap(); + /// assert!(consumed <= bytes.len()); + /// ``` fn from_bytes(bytes: &[u8]) -> Result<(Self, usize), DecodeError> where Self: Sized, diff --git a/src/preamble.rs b/src/preamble.rs index 937da433..cd94093a 100644 --- a/src/preamble.rs +++ b/src/preamble.rs @@ -58,7 +58,36 @@ where /// # Errors /// /// Returns a [`DecodeError`] if decoding the preamble fails or an -/// underlying I/O error occurs while reading from `reader`. +/// Asynchronously reads and decodes a preamble of type `T` from an async reader using bincode. +/// +/// Attempts to decode a value of type `T` from the beginning of the byte stream, reading more bytes as needed until decoding succeeds or an error occurs. Any bytes remaining after the decoded value are returned as leftovers. +/// +/// # Returns +/// +/// A tuple containing the decoded value and a vector of leftover bytes following the decoded preamble. +/// +/// # Errors +/// +/// Returns a `DecodeError` if decoding fails or if an I/O error occurs while reading from the reader. +/// +/// # Examples +/// +/// ``` +/// use tokio::io::BufReader; +/// use bincode::BorrowDecode; +/// +/// #[derive(Debug, PartialEq, bincode::BorrowDecode)] +/// struct MyPreamble(u8); +/// +/// #[tokio::main] +/// async fn main() { +/// let data = bincode::encode_to_vec(MyPreamble(42), bincode::config::standard()).unwrap(); +/// let mut reader = BufReader::new(&data[..]); +/// let (preamble, leftover) = read_preamble::<_, MyPreamble>(&mut reader).await.unwrap(); +/// assert_eq!(preamble.0, 42); +/// assert!(leftover.is_empty()); +/// } +/// ``` pub async fn read_preamble(reader: &mut R) -> Result<(T, Vec), DecodeError> where R: AsyncRead + Unpin, diff --git a/src/server.rs b/src/server.rs index fbc82842..87e6a5c2 100644 --- a/src/server.rs +++ b/src/server.rs @@ -322,6 +322,9 @@ where } #[allow(clippy::type_complexity)] +/// Runs a worker task that accepts incoming TCP connections and processes them asynchronously. +/// +/// Each accepted connection is handled in a separate task, with optional callbacks for preamble decode success or failure. The worker listens for shutdown signals to terminate gracefully. Accept errors are retried with exponential backoff. async fn worker_task( listener: Arc, factory: F, @@ -356,6 +359,27 @@ async fn worker_task( } #[allow(clippy::type_complexity)] +/// Processes an incoming TCP stream by decoding a preamble and dispatching the connection to a `WireframeApp`. +/// +/// Attempts to asynchronously decode a preamble of type `T` from the provided stream. If decoding succeeds, invokes the optional success handler, wraps the stream to include any leftover bytes, and passes it to a new `WireframeApp` instance for connection handling. If decoding fails, invokes the optional failure handler and closes the connection. +/// +/// # Type Parameters +/// +/// - `F`: A factory closure that produces `WireframeApp` instances. +/// - `T`: The preamble type, which must support borrowed decoding via the `Preamble` trait. +/// +/// # Examples +/// +/// ```no_run +/// # use std::sync::Arc; +/// # use mycrate::{process_stream, WireframeApp, Preamble}; +/// # use tokio::net::TcpStream; +/// # async fn example() { +/// let stream: TcpStream = /* ... */; +/// let factory = || WireframeApp::new(); +/// process_stream::<_, ()>(stream, factory, None, None).await; +/// # } +/// ``` async fn process_stream( mut stream: tokio::net::TcpStream, factory: F,