Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LocalWaker drops itself when cloning #52629

Closed
Thomasdezeeuw opened this issue Jul 22, 2018 · 0 comments
Closed

LocalWaker drops itself when cloning #52629

Thomasdezeeuw opened this issue Jul 22, 2018 · 0 comments

Comments

@Thomasdezeeuw
Copy link
Contributor

Running the following code:

#![feature(futures_api)]

use std::ptr::NonNull;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::task::{LocalWaker, Waker, UnsafeWake};

pub fn new_waker() -> LocalWaker {
    let waker = Box::new(MyWaker { count: AtomicUsize::new(0) });
    let ptr: *const dyn UnsafeWake = Box::into_raw(waker);
    let ptr = unsafe { NonNull::new_unchecked(ptr as *mut _) };
    unsafe { LocalWaker::new(ptr) }
}

#[derive(Debug)]
struct MyWaker {
    count: AtomicUsize,
}

unsafe impl UnsafeWake for MyWaker {
    unsafe fn clone_raw(&self) -> Waker {
        println!("clone_raw");
        let ptr: *const UnsafeWake = self;
        let ptr = NonNull::new_unchecked(ptr as *mut _);
        self.count.fetch_add(1, Ordering::SeqCst);
        Waker::new(ptr)
    }

    unsafe fn drop_raw(&self) {
        println!("drop_raw");
        if self.count.fetch_sub(1, Ordering::SeqCst) == 1 {
            // TODO: actually drop self.
        }
    }

    unsafe fn wake(&self) {
        println!("wake");
    }
}

fn main() {
    let local_waker1 = new_waker();
    let local_waker2  = local_waker1.clone();

    local_waker2.wake();

    drop(local_waker1);
    drop(local_waker2);
}

Prints the following:

clone_raw
drop_raw                     # The problem.
wake
drop_raw
drop_raw

It has an extra call to drop_raw.

I think it's because of the Clone implementation of LocalWaker:

unsafe {
        LocalWaker { inner: self.inner.as_ref().clone_raw().inner }
}

As UnsafeWake::clone_raw returns an Waker the LocalWaker code just takes the inner value. However because of this the Waker will be dropped and hence the erroneous extra call to drop_raw.

kennytm added a commit to kennytm/rust that referenced this issue Jul 24, 2018
… r=cramertj

Forget Waker when cloning LocalWaker

Since NonNull is Copy the inner field of the cloned Waker was copied for
use in the new LocalWaker, however this left Waker to be dropped. Which
means that when cloning LocalWaker would also erroneously call drop_raw.

This change forgets the Waker, rather then dropping it, leaving the
inner field to be used by the returned LocalWaker.

Closes rust-lang#52629.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant