Skip to content

Commit

Permalink
Remove _dropck flags
Browse files Browse the repository at this point in the history
The nomicon has been outdated about this since 2015,
see rust-lang/nomicon#363 for the update.
We don't need to tell dropck that we are owning a T as it
can already infer that from the generic parameter in the drop impl.
  • Loading branch information
Noratrieb committed Aug 14, 2022
1 parent 3b27d35 commit ae5a922
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 49 deletions.
41 changes: 1 addition & 40 deletions src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use super::{dealloc, Channel};
use core::fmt;
use core::marker::PhantomData;
use core::mem;
use core::ptr::NonNull;

Expand All @@ -10,41 +9,6 @@ use core::ptr::NonNull;
/// The message that could not be sent can be retreived again with [`SendError::into_inner`].
pub struct SendError<T> {
channel_ptr: NonNull<Channel<T>>,
/// Required due to the reasons outlined in
/// [this section](https://doc.rust-lang.org/nomicon/dropck.html) of the nomicon, as well as
/// the next section.
///
/// Without the phantom data, the following code would incorrectly compile. This code is
/// invalid because `error` gets dropped first (struct fields are dropped in declaration order)
/// but `oof` then accesses the data deallocated by `error`, causing a use-after-free.
///
/// ```compile_fail
/// let (tx, rx) = oneshot::channel::<Box<u8>>();
/// drop(rx);
/// let error = tx.send(Box::new(0)).unwrap_err();
///
/// struct Oof<'a>(&'a u8);
///
/// impl<'a> Drop for Oof<'a> {
/// fn drop(&mut self) {
/// println!("{}", self.0);
/// }
/// }
///
/// struct Foo<'a> {
/// error: SendError<Box<u8>>,
/// oof: Option<Oof<'a>>,
/// }
///
/// let mut foo = Foo {
/// error,
/// oof: None
/// };
///
/// foo.oof = Some(Oof(&**foo.error.as_inner()));
/// drop(foo);
/// ```
_dropck: PhantomData<T>,
}

unsafe impl<T: Send> Send for SendError<T> {}
Expand All @@ -58,10 +22,7 @@ impl<T> SendError<T> {
/// pointer is not used in a way which would violate this ownership transfer. Moreover,
/// the caller must assert that the channel contains a valid, initialized message.
pub(crate) const unsafe fn new(channel_ptr: NonNull<Channel<T>>) -> Self {
Self {
channel_ptr,
_dropck: PhantomData,
}
Self { channel_ptr }
}

/// Consumes the error and returns the message that failed to be sent.
Expand Down
10 changes: 1 addition & 9 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,8 @@ pub fn channel<T>() -> (Sender<T>, Receiver<T>) {
Sender {
channel_ptr,
_invariant: PhantomData,
_dropck: PhantomData,
},
Receiver {
channel_ptr,
_dropck: PhantomData,
},
Receiver { channel_ptr },
)
}

Expand All @@ -235,17 +231,13 @@ pub struct Sender<T> {
// If this type were covariant then we could safely extend lifetimes, which is not okay.
// Hence, we enforce invariance.
_invariant: PhantomData<fn(T) -> T>,
// See SendError for details
_dropck: PhantomData<T>,
}

#[derive(Debug)]
pub struct Receiver<T> {
// Covariance is the right choice here. Consider the example presented in Sender, and you'll
// see that if we replaced `rx` instead then we would get the expected behavior
channel_ptr: NonNull<Channel<T>>,
// See SendError for details
_dropck: PhantomData<T>,
}

unsafe impl<T: Send> Send for Sender<T> {}
Expand Down

0 comments on commit ae5a922

Please sign in to comment.