Generator's Send trait should have bounds

[ammaraskar]

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed that Generator implements Send as long as the closure has a static lifetime. However, this should also probably be bounded by T: Send, otherwise it's possible to smuggle across non-Send types across thread boundaries.

Here's an example of a data race in safe Rust code through a Generator.

#![forbid(unsafe_code)]

use generator::Gn;
use std::{rc::Rc, thread};

fn main() {
    let rc = Rc::new(());

    let rc_clone = rc.clone();
    let mut generator = Gn::new_scoped(move |_| {
        return rc_clone;
    });

    let child = thread::spawn(move || {
        let smuggled_rc = generator.next().unwrap();

        println!("RC in thread: {:p}", smuggled_rc);
        for _ in 0..1000000000 {
            let x = smuggled_rc.clone();
        }
    });

    println!("RC in main: {:p}", rc);
    for _ in 0..1000000000 {
        let x = rc.clone();
    }

    child.join().unwrap();
    assert_eq!(Rc::strong_count(&rc), 2);
}

Output:

RC in main: 0x5587d9ed6a50
RC in thread: 0x5587d9ed6a50

Return Code: -4 (SIGILL)

[Xudong-Huang]

Thanks for this info! I will try to fix that.

[Xudong-Huang]

I think this fix would break some existing code for the more strict bound check. I will create a 0.7.0 release for the changes.

[Xudong-Huang]

1. The functor that passed into the creation determines whether the generator could be send. If the functor is not send, then the generator is not send.
2. The argument type should also be send for that the generator could hold it for any usage
3. The return type should also be send, the generator could move to another thread, must make sure its return value is send.

We have to do more complex checks to determine if the generator is send. The implementation is not that easy.

[Xudong-Huang]

a new release 0.7 should be published soon

[zetanumbers]

Sadly sendable Generator is currently unsound because thread local variables may leak non-sendable values from within the our coroutine.

See https://blaz.is/blog/post/lets-pretend-that-task-equals-thread/ and https://users.rust-lang.org/t/controversial-opinion-keeping-rc-across-await-should-not-make-future-send-by-itself/100554/12 for details.

Opened separate #58