aimdb-dev / aimdb-dev/aimdb

[BUG] UDS Connection::recv is not cancel-safe — both session loops drop it mid-frame

Open
#202 1 comment 0 reactions 1 assignee View on GitHub

@thaodt is already working on this.

Since Jul 23, 2026.

⚡ async 🏗️ core bug
Dominant language
Rust
Stars
100
Forks
18
Avg merge
19h 47m
Merged PRs (30d)
33

Description

Verified still reproducing on f1fea6c (2026-09-16). Nothing has been
fixed. The server-side loss is now reproduced over a real socket, the client
loop turns out to have the same shape, and the fix originally proposed here
would not have worked — see Fix. References are refreshed after
#248/#250 moved TCP onto core's FramedConnection. Claim-by-claim check in
Verification.

Summary

Both session loops rebuild Connection::recv() on every select_biased!
iteration and drop it whenever another arm wins, so recv must be cancel-safe.
TCP, serial and WebSocket are; UDS is not. A UDS frame that needs more than one
read, arriving while another arm becomes ready, is cut in half and lost without
a trace.

The mechanism

Server — aimdb-core/src/session/server.rs:130-153:

let step = {
    let mut recv = conn.recv().fuse();
    let mut event = core::pin::pin!(event_rx.recv().fuse());
    select_biased! {
        r  = recv  => ...,
        ev = event => ...,
        sub_id = subs.select_next_some() => ...,
    }
};

Client — drive_connection in aimdb-core/src/session/client.rs:627-661 has the
same shape, with keepalive, prune and caller cmd arms beside recv.

select_biased! only prioritizes among futures that are ready. If recv is
Pending and another arm is Ready, the select resolves on that arm and recv — a
local — is dropped at the end of the block. The next iteration builds a fresh
one. The Connection trait (aimdb-core/src/session/mod.rs:376-378) does not
state the requirement.

Transport-by-transport

Transport recv impl Cancel-safe?
TCP, serial core's FramedConnection (aimdb-core/src/session/io.rs:281) — partial bytes live in the connection's Framer; ByteStream::read is cancel-safe on the shipped adapters (io.rs:66-72) yes
WebSocket server (aimdb-websocket-connector/src/transport.rs:73) pending queue on self, then self.ws.recv() yes
WebSocket client (transport.rs:144) self.ws.next() yes
UDS (aimdb-uds-connector/src/transport.rs:41) read_line into a local String no

The UDS impl:

fn recv(&mut self) -> BoxFut<'_, TransportResult<Option<Vec<u8>>>> {
    Box::pin(async move {
        let mut line = String::new();
        match self.reader.read_line(&mut line).await { ... }
    })
}

read_line consumes bytes from the BufReader into the future's own buffer
before the line is complete. Dropping the future discards them; the BufReader
has already advanced.

Impact

Server — reproduced. The request's head is lost. Its tail arrives as a frame
of its own, fails to decode (it carries the outer object's unmatched }), and is
skipped by the malformed-frame arm (server.rs:183-186). The session survives;
the request gets no reply and nothing is logged.

Client — by inspection. aimdb-client's AimxConnection dials unix://
endpoints through UdsDialer (aimdb-client/src/endpoint.rs:124), so the CLI
and anything else on that client are exposed the same way. A reply or event
arriving in pieces while the caller issues a command, the keepalive timer fires,
or a prune sweep runs is lost; the client also skips the malformed tail
(client.rs:737), so the call waits for a reply that never comes.

The trigger is a frame larger than one read — large record.set params, a
record.list over a big database, a large event payload — while the other
direction is active on the same connection. Rare, and hard to diagnose when it
does happen.

Fix

Either option stays inside aimdb-uds-connector.

Option A — move UDS onto FramedConnection. Design 052 already names this
route for the crate (docs/design/052-runtime-neutral-connectors.md:258). Add
an NDJSON Framer (split on \n, keep today's trailing-\r tolerance; a bad
line is FrameFault::Recoverable, since the next newline resyncs) and build
connections through it, as TCP does with FramingListener/FramingDialer.
Partial lines then live in the framer, and UDS stops being the one hand-rolled
transport. TokioByteStream<UnixStream> already satisfies ByteStream, but
aimdb-tokio-adapter is only a dev-dependency here — add it, or wrap
UnixStream in a local newtype.

Option B — minimal. Keep the hand-rolled connection and hoist the partial
line into it:

pub struct UdsConnection {
    reader: BufReader<OwnedReadHalf>,
    writer: OwnedWriteHalf,
    pending: Vec<u8>, // survives a dropped recv future
    peer: PeerInfo,
}

Read with read_until(b'\n', &mut self.pending) and take the buffer only once
it ends in a newline (or at EOF).

It must be read_until into a Vec<u8>, not read_line into a String.
This issue originally proposed a pending: String resumed with read_line.
That would not fix it: tokio moves the String's contents into the future and,
if the future is dropped, leaves the String empty. Tokio documents read_line
as not cancel-safe for exactly this reason and read_until as resumable.

Also worth doing

State the requirement on Connection::recv (aimdb-core/src/session/mod.rs:376-378):
both loops drop the future whenever another arm wins. ByteStream already
carries equivalent wording (io.rs:66-72) to mirror.

Test

Deterministic, in aimdb-uds-connector/tests/, against the real UdsServer:

  1. Subscribe to a SpmcRing record with with_remote_access().
  2. Write the first bytes of a record.list request, no newline; pause so the
    server reads them.
  3. Produce one value — the event arm wins the select.
  4. Read the event, write the rest of the request, and assert the reply arrives.

Keep the control case alongside: the same split request with no event in between
must be answered, which shows it is the event, not the split, that loses it.
A client-side counterpart needs a raw UnixListener peer that writes half a
reply while the caller sends a second command.


Verification

Checked against f1fea6c on 2026-09-16. The test above was written as a scratch
integration test in aimdb-uds-connector/tests/ and run five times, with the
same result each time:

control  (split, no event)   -> {"t":"reply","id":2,"ok":[…]}
event while head is buffered -> {"t":"event","seq":1,"sub":"1","data":{"n":1}}
split request after event    -> (no reply within 2 s)
whole request afterwards     -> {"t":"reply","id":4,"ok":[…]}

Claim by claim:

Claim as filed Status
run_session drops recv when another arm wins holds — server.rs:130-153, unchanged
UDS recv uses read_line into a local String holds — transport.rs:41-57, unchanged
Truncated frame is skipped, the session survives holds — reproduced; arm now at server.rs:183-186
The remainder "can mis-decode too" dropped — the tail always fails to decode and is skipped
TCP is cancel-safe via self.acc in tokio_transport.rs:59 outdated — file removed in #248/#250; TCP and serial use FramedConnection, still cancel-safe
WebSocket recv at transport.rs:140 moved — server :73, client :144; both still cancel-safe
Fix: resume read_line into a pending: String would not fix it — see Fix
No mention of cancel safety anywhere in session/ outdated — ByteStream documents it (io.rs:66-72); Connection still does not
Connection::recv at session/mod.rs:345 moved — :376-378
Only the server loop is affected incomplete — client drive_connection (client.rs:627-661) has the same shape
Found on feat/retire-aimdb-ws-protocol merged as #201; the bug survived the merge

Verification section generated by Claude Code

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.