Skip to content

Bound stream Send impls on the handle's Target, not the handle - #134

Open
cf-rhett wants to merge 1 commit into
jamesmunns:mainfrom
cf-rhett:stream-send-bounds
Open

Bound stream Send impls on the handle's Target, not the handle#134
cf-rhett wants to merge 1 commit into
jamesmunns:mainfrom
cf-rhett:stream-send-bounds

Conversation

@cf-rhett

Copy link
Copy Markdown

Summary

The four stream types all store Q::Target, but their Send impls were written against Q:

unsafe impl<Q: BbqHandle + Send> Send for StreamProducer<Q> {}

BbqHandle is a public, unsealed, safe trait with an associated Target, so a downstream impl is free to pair a Send handle with a !Send target. Q: Send then says nothing about the thing actually being stored, and you get a Send producer holding an Rc. framed.rs already gets this right — FramedGrantW and FramedGrantR are bounded on Q::Target: Send — so the two modules disagreed on the same question.

StreamProducer and StreamConsumer hold nothing but Q::Target. They never needed a manual impl; the derived one is already exactly right, so this deletes them. StreamGrantW and StreamGrantR also hold a NonNull, so they keep an unsafe impl, now bounded on Q::Target: Send to match framed.rs.

Only in-tree BbqHandle impls are &BBQueue and Arc<BBQueue>, both of which use type Target = Self — so nothing in the crate or its tests changes behavior, and the full matrix below still passes.

Reproducer

A purely safe downstream impl was enough to forge the bad bound. Against main this compiles and passes; with this change it fails to compile with Rc<BBQueue<..>> cannot be sent between threads safely.

#[derive(Clone, Copy)]
struct SendyHandle;                 // ZST, so Send

thread_local! {
    static RING: Rc<Ring> = Rc::new(BBQueue::new());
}

impl BbqHandle for SendyHandle {
    type Target = Rc<Ring>;         // but the Target is not
    /* ... */
    fn bbq_ref(&self) -> Self::Target { RING.with(|r| r.clone()) }
}

assert_send::<StreamProducer<SendyHandle>>();   // compiles on main

I left this out of the tree since catching it needs a compile-fail harness (trybuild) that the crate does not currently pull in. Happy to add it if you would like the coverage.

Testing

  • cargo test --features=std
  • cargo build --no-default-features
  • cargo build --features=std
  • cargo build --no-default-features --target=thumbv6m-none-eabi
  • cargo build --target=thumbv6m-none-eabi --features=portable-atomic-unsafe-assume-single-core
  • ./miri.sh

Note on my other open PRs

Independent of #132 and #133 in substance. #133 merges with this cleanly. #132 collides textually only — it appends a test module directly below the StreamGrantR impl this rewrites, so whichever lands second wants a keep-both resolution rather than a semantic one.

`StreamProducer`, `StreamConsumer`, `StreamGrantW`, and `StreamGrantR` all store
`Q::Target`, but their `Send` impls required `Q: Send`. `BbqHandle` is a public
safe trait with an associated `Target`, so a downstream impl can pair a `Send`
handle with a `!Send` target and hand back a `Send` producer that holds an `Rc`.
`Q: Send` says nothing about `Q::Target`, and nothing else in the crate closes
that gap.

The producer and consumer hold nothing but `Q::Target`, so they need no manual
impl at all; the derived one is already exactly right. The two grants also hold
a `NonNull`, so they keep an unsafe impl, now bounded on `Q::Target: Send` the
way `FramedGrantW` and `FramedGrantR` already are.
@cf-rhett
cf-rhett marked this pull request as ready for review July 31, 2026 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant