Add support for scoped threads

[akonradi]

Add loom::thread::scope and loom::thread::Scope to mirror the API provided by the standard library for implementing scoped threads.

Fixes #308

[akonradi]

Ping for review?

[akonradi]

@taiki-e pinging you since you approved PR 311, which was in a similar area. Can you TAL or help me find a reviewer?

[taiki-e]

Thanks for the PR!

It is difficult to implement scoped threads correctly and this implementation is also unsound. Consider the following test case (based on https://github.com/crossbeam-rs/crossbeam/pull/844#issuecomment-1143288972).

// tests/thread_api.rs
#[test]
fn scoped_thread_t() {
    struct PrintOnDrop<'a>(&'a str);

    impl Drop for PrintOnDrop<'_> {
        fn drop(&mut self) {
            thread::yield_now();
            println!("{}", self.0);
        }
    }

    loom::model(|| {
        let a = "hello".to_string();
        let r = &a;
        thread::scope(|s| {
            s.spawn(move || PrintOnDrop(r));
        });
        drop(a);
    });
    std::thread::sleep(std::time::Duration::from_secs(1));
}

thread::scope must wait for all spawned threads to complete, but in the current implementation, thread::scope does not wait for the spawned thread, so the code in the spawned thread accesses the freed variable, or the program itself terminates before the spawned thread completes.

$ cargo test --package loom --test thread_api --all-features -- scoped_thread_t --exact --nocapture

running 1 test
`L�F
test scoped_thread_t ... ok

running 1 test
test scoped_thread_t ... ok

[akonradi]

Thank you very much for the review. This is my first serious foray into unsafe Rust, so I'm not at all surprised that what I wrote was unsound!

The example actually tickled a bug in the dropping of unused thread results. The spawned thread was completing but its result was being written to memory behind an Arc that wasn't dropped until after the main thread had been unblocked. I've pushed a fix with a regression test (that doesn't double-panic, as my naive s/println/assert_eq alteration of your test case did 😄).

[notgull]

Any chance this PR can be reviewed? It's blocking me adding loom support for crossbeam-queue.

[Darksonn]

The scope API here uses a lot of the loom internals directly. Would it be possible to move that part of the logic into a method analogous to spawn_unchecked, and then implement the scoped API on top of that? This way, the scoped API doesn't have to touch loom internals.

@notgull The loom crate has very little reviewer bandwidth, but if this will let you use it for crossbeam-queue, then I'm willing to prioritize it.

[akonradi]

I don't have time to work on this now, so if someone wants to take over and do the refactor, go for it!