Skip to content

Add sync buffered-read primitives to TdsPacketReader #261

Description

Part of #250.

Problem statement

TdsPacketReader exposes only async accessors. A decode path that wants to ask "is this row already fully resident in the receive buffer, and can I decode it without awaiting?" currently has no way to do so.

This is a prerequisite for the buffered two-pass decode work, and is worth landing separately so that change's diff stays confined to decode logic.

One complication to get right: the trait is defined twice under cfg gates —

  • mssql-tds/src/io/packet_reader.rs:13 under cfg(not(fuzzing))
  • mssql-tds/src/io/packet_reader.rs:46 under cfg(fuzzing)

— and there is a Box<dyn ...> blanket impl at :76. Any addition must be made in all three places or the fuzzing build breaks.

Proposed solution

Three sync methods, each with a declining default implementation so no existing implementor is forced to change:

fn buffered_slice(&self) -> Option<&[u8]> { None }
fn consume_buffered(&mut self, len: usize) -> bool { false }
fn try_read_byte(&mut self) -> Option<u8> { None }

The defaults decline, so an implementor with no buffer of its own simply never offers the fast path and callers transparently fall back to the async route. Only the network reader implements them for real.

consume_buffered returns bool rather than panicking, so that a mis-drive by a caller degrades to a declined fast path instead of a crash.

Affected crate

mssql-tds

Alternatives considered

  • A separate BufferedRead trait, downcast at the call site. Downcasting through &mut dyn TdsPacketReader is awkward, and the decode path already holds this trait object — adding a second one to reach the same reader is not worth it.
  • Make the methods required rather than defaulted. Would force every implementor, including the fuzz and mock readers, to write a decline stub for no benefit.
  • Expose the buffer directly as a field. Not possible through a trait object, and it would prevent the reader from maintaining its own consumed-offset invariant.

Additional context

Deliberately scoped narrow: purely additive, fully defaulted, no behaviour change until the buffered decode work consumes it.

Worth noting for anyone reading this trait: #[async_trait] is applied at :11, :44, and :75, which means every read_byte().await heap-allocates a boxed future. On the #238 benchmark shape that is on the order of 10^8 box allocations. Removing it is a genuinely separate axis — roughly 50 mechanical sites, 35 of them &mut (dyn TdsPacketReader + Send + Sync) in decoder.rs, plus Box<dyn> uses in fuzz_support.rs — and is explicitly not in scope here. Note the PoC in #238 does not remove it either.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions