task: remove wrong comments about non-existent LocalWake trait

[seanmonstar]

A LocalWaker is specifically !Send , and the unsafety comment around
LocalWaker::new only specifies that it be safe to call wake_local.
One could then accidentally promote a LocalWaker into a Waker, which
is universally Send, simply via Waker::from(local_waker). A
LocalWaker the was built expecting to not be Send, such as using
Rc, could be sent to other threads safely.

Separately, though somewhat related, Context holds a &LocalWaker
internally, and exposes a waker() -> &Waker method. This simply
transmutes the &LocalWaker to &Waker, which would be unsound, except
that you can't "send" a &Waker, you'd need to clone it first. Since
UnsafeWake::clone_raw requires that it return a Waker, the transmute
is not unsound. The transmuted LocalWaker will be promoted to a
Waker correctly.

That would mean that if UnsafeWake::clone_raw were to be changed, such
as returning Self instead of Waker, this would no longer be sound.
Thus, this also adds a comment to clone_raw to remember this.

r? @cramertj

[seanmonstar]

I also removed these comments, since there is no LocalWake trait.

[cramertj]

Good catch! Sorry, this got left in after I rewrote this code to work differently. I agree that this comment should go away, and it should instead suggest using the local_waker_from_nonlocal or local_waker functions in std::task.

[cramertj]

@seanmonstar Thanks for pointing out this potential source of confusion! I think this is a documentation issue-- perhaps you can help me figure out the best way to document the required usage. LocalWaker::new takes a NonNull<UnsafeWake>, and the UnsafeWake trait itself requires Send and Sync. Implementations of UnsafeWake methods other than wake_local are expected to be thread-safe.

[seanmonstar]

With UnsafeWake: Send + Sync, it seems that a user couldn't actually create a local waker that clones via Rc instead of Arc. In that case, I'm having a hard time seeing how LocalWaker could be useful then, unless users are expected to unsafe impl Send for MyWaker and promise themselves to never actually send it, which sounds like a very scary thing to encourage people to do.

[cramertj]

@seanmonstar

With UnsafeWake: Send + Sync, it seems that a user couldn't actually create a local waker that clones via Rc instead of Arc.

That's correct!

I'm having a hard time seeing how LocalWaker could be useful then, unless users are expected to unsafe impl Send for MyWaker and promise themselves to never actually send it, which sounds like a very scary thing to encourage people to do.

LocalWaker allows for unsynchronized access to memory inside of the waker that would otherwise have to be synchronized. For example:

struct MyOptimizedTaskQueue {
    local_queue: RefCell<VecDeque<Arc<MyTask>>>,
    sync_queue: crossbeam::sync::SegQueue<Arc<MyTask>>,
}

unsafe impl Send for MyOptimizedTaskQueue {}
unsafe impl Sync for MyOptimizedTaskQueue {}

struct MyTask {
    task: RefCell<TaskObj>,
    queue: Arc<MyOptimizedTaskQueue>,
}

unsafe impl Send for MyTask {}
unsafe impl Sync for MyTask {}

impl Wake for MyOptimizedTaskQueue {
    fn wake(arc_self: &Arc<Self>) {
        arc_self.queue.sync_queue.push(arc_self.clone());
    }

    // HERE is the key bit that `LocalWaker` allows-- because we know we're on the same thread
    // from which the `LocalWaker` was created, we know that we can use the `local_queue` `RefCell`
    // without any synchronization.
    unsafe fn wake_local(arc_self: &Arc<Self>) {
        arc_self.queue.local_queue.push(arc_self.clone());
    }
}

This gives you a single-threaded task system that only requires synchronization for Arc cloning-- not for wakeups nor for polling futures.

[seanmonstar]

O_O Being able to be certain that the waker is actually local seems very tricky; I'd be suspicious of code that thinks it's been done safely.

Anyways, seems the only thing then here is that the documentation mentions non-existent LocalWake, so I'll update.

[cramertj]

O_O Being able to be certain that the waker is actually local seems very tricky; I'd be suspicious of code that thinks it's been done safely.

Right-- you have to create it in one place, on one thread, and then it can be passed down into task::Context after calling the (unsafe) local_waker function. LocalWaker isn't Send or Sync, so this makes it impossible to misuse.

[cramertj]

@bors r+ rollup

[bors]

📌 Commit 4f4e91a has been approved by cramertj