RawCan: a blocking driver in Transmit stalls every subscription, because _pendingGate is held across it

[dborgards]

Follow-up to item 1 of #53, which was closed by #100 with this item deliberately not done. Reopening it as its own issue because it stopped being a minor finding once the counter-argument was written down: it is a lock-scope design change with a correctness constraint on both sides, not a one-line narrowing.

What happens

CanBusService.SendWithEchoConfirmAsync holds _pendingGate across _bus.Transmit(...). With a blocking vendor driver, the thread inside Transmit owns the lock for as long as the driver takes, and the adapter RX thread blocks in TryMatchEcho waiting for it. Every subscription stalls with it — including subscriptions that have nothing to do with the frame being sent.

The lock itself only needs to cover the pending-list bookkeeping. That was item 1's original argument and it is still correct as far as it goes.

Why it was refused, and what any fix has to preserve

Narrowing the lock naively breaks the ordering guarantee the FIFO depends on. From the rationale now in CanBusService.cs:

Register and transmit as one atomic step under _pendingGate: this is what makes TryMatchEcho's FIFO order equal actual transmission order rather than mere registration order. Without it, two threads sending byte-identical frames could register in one order but transmit in the other, so the oldest pending entry is not necessarily the oldest sent one — an echo could then confirm the wrong caller, or confirm a send before it was even transmitted (FR-RAW-031).

Three constraints, all of which a replacement must satisfy:

1. Echo matching must follow transmission order, not registration order. This is the one that kills the naive fix. Two byte-identical concurrent sends are indistinguishable to the matcher except by order.
2. A synchronous echo delivered inside Transmit must still work. CanKit.Adapter.Virtual in ChannelWorkMode.Echo re-enters the same lock on the same thread; Monitor is reentrant, so TryMatchEcho sees only this thread's own just-registered entry. Any replacement has to keep that path correct — and it is the path most of the test suite runs on.
3. The dispose race must stay closed. Dispose sets _pendingDisposed under this same lock, so a call cannot register after the sweep has cancelled every pending entry; it gets ObjectDisposedException instead, matching the eager check at the top of SendConfirmed.

Sketch of a fix that could satisfy all three

Take a monotonically increasing sequence number per PendingKey under the lock, release the lock, then transmit; TryMatchEcho matches the lowest unclaimed sequence rather than the head of a list. Transmission order and registration order can then diverge without the matcher caring, because the sequence was assigned before either.

That is a sketch, not a design. The open questions are what happens to a send whose Transmit throws after it took a sequence number (it must not leave a hole that blocks later matches), and whether the reentrant synchronous-echo path still resolves against the right entry when the lock is no longer held during the driver call.

Related coupling, worth solving in the same pass

The note on the cancellation callback in CanBusService.cs records a second consequence of the same design: a caller's SendConfirmed task completes only after the finally unlinks the entry, and that unlink takes _pendingGate. So completing a send is coupled to the lock too, not just matching. #100 removed the worst symptom — Cancel() no longer blocks on an unrelated send's driver call — but the coupling itself follows from holding the lock across Transmit and would go away with it.

Not urgent

No reported failure depends on this. It matters for real hardware adapters with blocking transmit paths, which the current test suite does not exercise — ControllableBus and CanKit.Adapter.Virtual both return promptly. Anyone picking this up should probably add a bus double whose Transmit blocks for a controllable duration first, and assert that an unrelated subscription keeps receiving while it does. That test is worth having even if the lock is never narrowed, because it pins the current behaviour honestly instead of leaving it undocumented.

Refs #53, #100, FR-RAW-031.

[dborgards]

Step one done in #124 — the blocking-transmit test this ticket asks for. Two things it turned up that change the terms of the rest.

1. The stall is narrower than stated here

Every subscription stalls with it — including subscriptions that have nothing to do with the frame being sent.

Measured rather than reasoned: OnFrameObserved calls TryMatchEcho — the only thing on the RX path that takes _pendingGate — only when the frame is flagged as an echo and a send is pending. Dispatch to subscriptions reads a volatile snapshot and takes no lock.

So while a driver blocks inside Transmit, a plain frame is delivered normally; an echo frame parks the RX thread. The blanket stall follows from that, because TryMatchEcho runs before dispatch on the same thread and a real adapter has one RX thread — so everything behind that echo queues. Both halves are asserted in #124.

It still matters, and the trigger is common on an echo-mode adapter. But "the RX thread parks on an echo frame" is the statement a fix has to address, and it is not the same statement as "every subscription stalls".

2. The sketch does not satisfy this ticket's own constraint 1

Take a monotonically increasing sequence number per PendingKey under the lock, release the lock, then transmit; TryMatchEcho matches the lowest unclaimed sequence […] Transmission order and registration order can then diverge without the matcher caring, because the sequence was assigned before either.

The sequence is assigned at registration, so matching follows registration order. Constraint 1 says that is exactly what must not happen:

Echo matching must follow transmission order, not registration order. […] Two byte-identical concurrent sends are indistinguishable to the matcher except by order.

Worked through: A registers (seq 1), B registers (seq 2), B transmits first, A second. The wire order is B, A; the echoes arrive B, A; the matcher gives the first echo to seq 1 = A. A is confirmed by B's frame — i.e. confirmed before its own was transmitted, which is the FR-RAW-031 failure the code comment names. HostTransmitTimestamp is unaffected (it is taken per caller after Transmit returns), so the damage is confined to when a caller believes its frame reached the wire — but that is what a transport layer starts a deadline from.

So the sketch trades the constraint away rather than satisfying it. Any design that keeps constraint 1 has to make the sequence reflect transmission order, which means something serialises transmission — and the one already written down is in CanBusService.cs:

If one ever has to be, the fix is a queue in front of the driver, not a pending list whose order no longer matches the wire.

A TX worker would satisfy all three constraints at once: transmissions are serialised by the queue so enqueue order is wire order; _pendingGate covers bookkeeping only; a synchronous echo arrives on the worker thread, which does not hold the lock; dispose is untouched. It also changes latency characteristics and who owns the caller's thread, which is a real design change rather than a lock-scope tweak.

What I would do with this ticket

Not narrow the lock on speculation. The evidence now says:

• no reported failure depends on it, as this ticket already noted;
• the affected path is echo-matching on an echo-mode adapter with a blocking driver — and CanBusService.cs argues such a driver is unusable with this service regardless, because ICanBus.Transmit is on every other consumer's dispatch path too;
• the cheap fix is unsound, and the sound fix is a TX queue.

So the choice is between closing this as documented-and-declined with #124 as the record, or keeping it open as the placeholder for a TX queue if a blocking vendor driver ever actually has to be supported. That is the maintainer's call, not mine — I am not touching the lock either way without a concrete adapter that needs it.

---

Generated by Claude Code

[dborgards]

Closing as documented and declined — maintainer's decision.

The record is #124: A_Blocking_Transmit_Does_Not_Hold_Up_A_Plain_Frame, plus the existing Frame_Arrival_Is_Stamped_Before_The_Pending_Send_Lock that already pins the echo stall. Between them the behaviour is asserted rather than left undocumented, which is what this ticket asked for as its first step.

Why not the rest:

• the affected path is echo-matching on an echo-mode adapter with a blocking driver, and CanBusService.cs argues such a driver is unusable with this service regardless, because ICanBus.Transmit sits on every other consumer's dispatch path too;
• the cheap fix is unsound — the sequence-number sketch in this ticket assigns the sequence at registration, which is the ordering constraint 1 forbids;
• the sound fix is a TX queue in front of the driver, which is an architecture change, not a lock-scope tweak.

If a blocking vendor driver ever has to be supported, reopen or file fresh: the constraints, the worked counter-example against the sketch, and the TX-queue shape are all in this comment.

---

Generated by Claude Code