smol-rs/async-task

Clarify safety of spawn_unchecked for Runnable lifetime

orlp opened this issue · 2 comments

orlp commented

async_task::spawn_unchecked mentions the following requirement:

If future is not 'static, borrowed variables must outlive its Runnable.

The problem is that I don't see a feasible way to guarantee this. Currently I have the following code:

fn ensure_send<T: Send>(val: T) -> T { val }
fn ensure_send_sync<T: Send + Sync>(val: T) -> T { val }

// SAFETY: the runner may not live longer than ScopedExecutor.
// We ensure this by keeping a weak reference to the scheduler
// which is destroyed when the executor is dropped.
// The other conditions are checked by ensure_send(_sync).
let scheduler = Arc::downgrade(&self.scheduler);
async_task::Builder::new()
    .metadata(TaskMetadata { affinity, priority })
    .spawn_unchecked(
        ensure_send(|_| fut),
        ensure_send_sync(move |task| if let Some(s) = scheduler.upgrade() {
            s.add_task(task)
        }),
    );

I believe (and please do correct me if I'm wrong) this is an intended usage of spawn_unchecked to support a potential ScopedExecutor<'a> which can add fut: Future<...> + Send + 'a tasks. If the ScopedExecutor is dead, any existing Wakers will call schedule which does nothing as the weak pointer is dead.

However... this technically violates the safety requirement of spawn_unchecked! Because my schedule functions gets called... with a Runnable! And nothing is preventing fut from storing any Waker it receives in its poll function inside some &'static Mutex<Waker>, which it can call way after 'a has expired. Sure, I immediately drop it since the weak pointer is dead, but technically a Runnable has survived after 'a.

orlp commented

Thinking about it some more, what I did isn't sound regardless. The Future could have a Drop impl that relies on lifetime 'a, but it could be dropped much later than that. I need to have the actual future itself behind a Weak reference inside a Waker, so I don't think I can use async-task, unfortunately.

orlp commented

Hmm, on further thought, it seems as long as I cancel all tasks before scope 'a ends (using a closure trick similar to std::thread::scoped) all tasks will get scheduled one last time allowing me to drop all runnables.