Rework ScopedCoroutine by using a scope in addition to Pin

[Amanieu]

Fixes #59

[zetanumbers]

ScopedCoroutine is Unpin

use std::{mem, sync::mpsc};

use corosensei::ScopedCoroutine;

fn main() {
    ScopedCoroutine::new(
        |_, ()| (),
        |outer_coroutine| {
            let mut s = "Happy".to_owned();
            let (tx1, rx1) = mpsc::sync_channel(1);
            let (tx2, rx2) = mpsc::sync_channel(1);
            {
                let s = s.as_str();
                ScopedCoroutine::new(
                    |yielder, ()| {
                        std::thread::scope(move |scope| {
                            let _thrd = scope.spawn(move || {
                                rx1.recv().unwrap();
                                tx2.send(s.to_owned()).unwrap();
                            });
                            let () = yielder.suspend(());
                        });
                    },
                    |mut coroutine| {
                        coroutine.as_mut().resume(());
                        mem::swap(coroutine.get_mut(), outer_coroutine.get_mut());
                    },
                );
            }
            s.clear();
            s.push_str("Sad");
            tx1.send(()).unwrap();
            let data_race_s = rx2.recv().unwrap();
            dbg!(&s, &data_race_s);
        },
    );
}

Prints out:

[src/main.rs:34:13] &s = "Sad"
[src/main.rs:34:13] &data_race_s = "Sadpy"

[Amanieu]

Added PhantomPinned.

[zetanumbers]

This variant is not as flexible as my suggestion for Input and Yield as those have to strictly outlive a scope closure. It's also strange that you do not spawn coroutines from inside the scope closure, like with thread::scope. Personally I would have copied that API instead of using Pin, although that could require Input: 'static or Yield: 'static.

But you have effectively almost recreated thread::scope's lifetime job there. Scope closure can only work with Input and Yield though &mut Self, which makes those types invariant (thread::scope's 'env) and thus cannot be subtyped to use lifetime from inside of the scope closure. So non-static Input and Yield shouldn't be unsafe to use. It also implicitly introduces similar higher-ranked lifetime (thread::scope's 'scope) as part of S: FnOnce bound. However there's one last difference left, it might not be true that 'env: 'scope, which I find concerning.

I don't think this is unsound.

[zetanumbers]

LGTM i guess

[Amanieu]

The problem with a thread::scope API is that it requires dynamic allocation of coroutines in the Scope context. With this design the ScopedCoroutine is allocated on the stack with no dynamic memory allocation.

I do think that the API is quite ugly to use though, I'll try and see if it can be improved.

[zetanumbers]

You know, this looks like rayon::join but coroutine version. It has symmetry between the scope closure and the coroutine body, kinda reminiscent of #42. By this analogy it must be sound. Perhaps exploiting this symmetry may yield better looking API.

[Amanieu]

I reworked the API to separate creating the coroutine from executing it in a scope:

use corosensei::ScopedCoroutine;

let mut counter = 0;
let coroutine = ScopedCoroutine::new(|yielder, input| {
    if input == 0 {
        return;
    }
    counter += input;
    loop {
        let input = yielder.suspend(());
        if input == 0 {
            break;
        }
        counter += input;
    }
});
coroutine.scope(|mut coroutine| {
    coroutine.as_mut().resume(1);
    coroutine.as_mut().resume(2);
    coroutine.as_mut().resume(3);
    coroutine.as_mut().resume(4);
    coroutine.as_mut().resume(5);
});
assert_eq!(counter, 15);

[zetanumbers]

Lifetime of closure F is erased:

use std::sync::mpsc;

use corosensei::ScopedCoroutine;

fn main() {
    let mut s = "Happy".to_owned();
    let (tx1, rx1) = mpsc::sync_channel(1);
    let (tx2, rx2) = mpsc::sync_channel(1);
    {
        let s1 = s.as_str();
        let coroutine = ScopedCoroutine::new(|yielder, ()| {
            std::thread::scope(move |scope| {
                let _thrd = scope.spawn(move || {
                    rx1.recv().unwrap();
                    tx2.send(s1.to_owned()).unwrap();
                });
                let () = yielder.suspend(());
            });
        });
        coroutine.scope(|mut coroutine| {
            coroutine.resume(());
            s.clear();
            s.push_str("Sad");
            tx1.send(()).unwrap();
            let data_race_s = rx2.recv().unwrap();
            dbg!(&s, &data_race_s);
        });
    }
}

Prints out:

[src/main.rs:26:13] &s = "Sad"
[src/main.rs:26:13] &data_race_s = "Sadpy"

[Amanieu]

Should hopefully be fixed now.