Problem to solve
The ownership and lifetime rules for CanFrame and its backing memory are currently implicit, and a few concrete defects follow from that:
CanFrame.Dispose() always disposes the backing IMemoryOwner<byte>, even for frames that do not own their memory (e.g. frames wrapping a slice of a larger buffer). Disposing such a frame tears down memory it does not own.
VirtualBusHub.Broadcast hands the same shared frame/memory owner to every recipient (and to the echo path). If one consumer disposes its copy early, the memory of every other consumer — and of the sender's echo — is invalidated (use-after-free / double-dispose territory).
- The echoed frame on the Virtual bus is not marked
IsEcho = true.
VirtualBusHub leaks hub instances: hubs for a session accumulate instead of being removed once the last member leaves.
Proposal
Define and enforce an explicit ownership contract:
CanFrame.Dispose() respects an OwnMemory flag: frames that do not own their memory treat Dispose() as a documented no-op.
- New primitive
CanFrame.Clone(IBufferAllocator) producing an independent copy backed by a freshly rented buffer, decoupling the copy's Dispose lifetime from the source frame's memory owner.
VirtualBusHub.Broadcast gives each recipient its own clone, so one consumer can never invalidate another consumer's (or the sender's) memory; the echo frame is correctly marked IsEcho = true.
- Hub join/detach is routed through a single registry lock so a session hub is atomically removed when its last member leaves.
This touches core semantics (CanFrame.Dispose behavior), so it is potentially breaking for code that relied on the old dispose-always behavior. That is why I am opening this as a discussion first rather than a surprise PR: does upstream want this contract, and if yes, in which release line (behavior fix vs. documented breaking change)?
Alternatives considered
- Document the pitfalls only: leaves the use-after-free/double-dispose defects in place; the Virtual bus is the default hardware-free backend used in tests and samples, so the shared-owner broadcast is easy to hit.
- Reference-counted shared owner: more machinery than the contract needs; per-recipient clones are cheap at CAN frame sizes.
Areas
Reference implementation
We have been running this contract in our fork, including regression coverage for the Dispose/Clone ownership semantics and a Virtual-loopback multi-consumer/echo/hub-leak test suite: dborgards@242f4ea
Happy to open a PR once there is agreement on the contract and the targeted release line.
Problem to solve
The ownership and lifetime rules for
CanFrameand its backing memory are currently implicit, and a few concrete defects follow from that:CanFrame.Dispose()always disposes the backingIMemoryOwner<byte>, even for frames that do not own their memory (e.g. frames wrapping a slice of a larger buffer). Disposing such a frame tears down memory it does not own.VirtualBusHub.Broadcasthands the same shared frame/memory owner to every recipient (and to the echo path). If one consumer disposes its copy early, the memory of every other consumer — and of the sender's echo — is invalidated (use-after-free / double-dispose territory).IsEcho = true.VirtualBusHubleaks hub instances: hubs for a session accumulate instead of being removed once the last member leaves.Proposal
Define and enforce an explicit ownership contract:
CanFrame.Dispose()respects anOwnMemoryflag: frames that do not own their memory treatDispose()as a documented no-op.CanFrame.Clone(IBufferAllocator)producing an independent copy backed by a freshly rented buffer, decoupling the copy's Dispose lifetime from the source frame's memory owner.VirtualBusHub.Broadcastgives each recipient its own clone, so one consumer can never invalidate another consumer's (or the sender's) memory; the echo frame is correctly markedIsEcho = true.This touches core semantics (
CanFrame.Disposebehavior), so it is potentially breaking for code that relied on the old dispose-always behavior. That is why I am opening this as a discussion first rather than a surprise PR: does upstream want this contract, and if yes, in which release line (behavior fix vs. documented breaking change)?Alternatives considered
Areas
Reference implementation
We have been running this contract in our fork, including regression coverage for the Dispose/Clone ownership semantics and a Virtual-loopback multi-consumer/echo/hub-leak test suite: dborgards@242f4ea
Happy to open a PR once there is agreement on the contract and the targeted release line.