Skip to content

Commit

Permalink
Replace transmute with Thread::{into,from}_raw
Browse files Browse the repository at this point in the history
Makes things a little safer.
  • Loading branch information
Thomasdezeeuw committed Dec 21, 2024
1 parent 89052df commit a90ddcd
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
//! This crate has one optional feature: `test`. The `test` feature will enable
//! the `test` module which contains testing facilities.
#![feature(doc_auto_cfg, doc_cfg_hide, never_type)]
#![feature(doc_auto_cfg, doc_cfg_hide, never_type, thread_raw)]
#![warn(
anonymous_parameters,
bare_trait_objects,
Expand Down
37 changes: 22 additions & 15 deletions src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@
//! ```
use std::future::Future;
use std::io;
use std::panic::{self, AssertUnwindSafe};
use std::pin::pin;
use std::task::{self, Poll, RawWaker, RawWakerVTable};
use std::thread::{self, Thread};
use std::time::{Duration, Instant};
use std::{io, ptr};

use heph_inbox::Receiver;
use heph_inbox::{self as inbox, ReceiverConnected};
Expand Down Expand Up @@ -438,9 +438,7 @@ impl SyncWaker {

/// Returns itself as `task::RawWaker` data.
fn into_data(self) -> *const () {
// SAFETY: this is not safe. This only works because `Thread` uses
// `Pin<Arc<_>>`, which is a pointer underneath.
unsafe { std::mem::transmute(self) }
self.handle.into_raw()
}

/// Inverse of [`SyncWaker::into_data`].
Expand All @@ -449,16 +447,25 @@ impl SyncWaker {
///
/// `data` MUST be created by [`SyncWaker::into_data`].
unsafe fn from_data(data: *const ()) -> SyncWaker {
// SAFETY: inverse of `into_data`, see that for more info.
unsafe { std::mem::transmute(data) }
SyncWaker {
// SAFETY: caller must ensure that `data` is created by
// `SyncWaker::into_data`, which forfills all requirements for
// `Thread::from_raw`.
handle: unsafe { Thread::from_raw(data) },
}
}

/// Same as [`SyncWaker::from_data`], but returns a reference instead of an
/// owned `SyncWaker`.
unsafe fn from_data_ref(data: &*const ()) -> &SyncWaker {
// SAFETY: inverse of `into_data`, see that for more info, also see
// `from_data`.
&*(ptr::from_ref(data).cast())
/// Same as [`SyncWaker::from_data`], but doesn't invalidates `data`.
unsafe fn from_data_ref(data: *const ()) -> SyncWaker {
// SAFETY: THIS IS INCORRECT.
//
// If anything between `SyncWaker::from_data` and `waker.into_waker`
// panics this will go badly.
let waker = unsafe { SyncWaker::from_data(data) };
let clone = waker.clone();
let out = waker.into_data();
assert!(out == data); // Need to ensure that `data` is valid.
clone
}

const VTABLE: RawWakerVTable = RawWakerVTable::new(
Expand All @@ -469,8 +476,8 @@ impl SyncWaker {
);

unsafe fn clone(data: *const ()) -> RawWaker {
let waker = SyncWaker::from_data_ref(&data);
let data = waker.clone().into_data();
let waker = SyncWaker::from_data_ref(data);
let data = waker.into_data();
RawWaker::new(data, &SyncWaker::VTABLE)
}

Expand All @@ -479,7 +486,7 @@ impl SyncWaker {
}

unsafe fn wake_by_ref(data: *const ()) {
SyncWaker::from_data_ref(&data).handle.unpark();
SyncWaker::from_data_ref(data).handle.unpark();
}

unsafe fn drop(data: *const ()) {
Expand Down

0 comments on commit a90ddcd

Please sign in to comment.