Skip to content

Commit

Permalink
Unrolled build for rust-lang#121622
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#121622 - dtolnay:wake, r=cuviper

Preserve same vtable pointer when cloning raw waker, to fix Waker::will_wake

Fixes rust-lang#121600.

As `@jkarneges` identified in rust-lang#121600 (comment), the issue is two different const promotions produce two statics at different addresses, which may or may not later be deduplicated by the linker (in this case not).

Prior to rust-lang#119863, the content of the statics was compared, and they were equal. After, the address of the statics are compared and they are not equal.

It is documented that `will_wake` _"works on a best-effort basis, and may return false even when the Wakers would awaken the same task"_ so this PR fixes a quality-of-implementation issue, not a correctness issue.
  • Loading branch information
rust-timer authored Mar 3, 2024
2 parents 5119208 + db535ba commit 020de25
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 0 deletions.
13 changes: 13 additions & 0 deletions library/alloc/src/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,15 @@ impl<W: Wake + Send + Sync + 'static> From<Arc<W>> for RawWaker {
#[inline(always)]
fn raw_waker<W: Wake + Send + Sync + 'static>(waker: Arc<W>) -> RawWaker {
// Increment the reference count of the arc to clone it.
//
// The #[inline(always)] is to ensure that raw_waker and clone_waker are
// always generated in the same code generation unit as one another, and
// therefore that the structurally identical const-promoted RawWakerVTable
// within both functions is deduplicated at LLVM IR code generation time.
// This allows optimizing Waker::will_wake to a single pointer comparison of
// the vtable pointers, rather than comparing all four function pointers
// within the vtables.
#[inline(always)]
unsafe fn clone_waker<W: Wake + Send + Sync + 'static>(waker: *const ()) -> RawWaker {
unsafe { Arc::increment_strong_count(waker as *const W) };
RawWaker::new(
Expand Down Expand Up @@ -304,6 +313,10 @@ impl<W: LocalWake + 'static> From<Rc<W>> for RawWaker {
#[inline(always)]
fn local_raw_waker<W: LocalWake + 'static>(waker: Rc<W>) -> RawWaker {
// Increment the reference count of the Rc to clone it.
//
// Refer to the comment on raw_waker's clone_waker regarding why this is
// always inline.
#[inline(always)]
unsafe fn clone_waker<W: LocalWake + 'static>(waker: *const ()) -> RawWaker {
unsafe { Rc::increment_strong_count(waker as *const W) };
RawWaker::new(
Expand Down
2 changes: 2 additions & 0 deletions library/alloc/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#![feature(thin_box)]
#![feature(strict_provenance)]
#![feature(drain_keep_rest)]
#![feature(local_waker)]
#![allow(internal_features)]
#![deny(fuzzy_provenance_casts)]
#![deny(unsafe_op_in_unsafe_fn)]
Expand All @@ -62,6 +63,7 @@ mod rc;
mod slice;
mod str;
mod string;
mod task;
mod thin_box;
mod vec;
mod vec_deque;
Expand Down
34 changes: 34 additions & 0 deletions library/alloc/tests/task.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
use alloc::rc::Rc;
use alloc::sync::Arc;
use alloc::task::{LocalWake, Wake};
use core::task::{LocalWaker, Waker};

#[test]
fn test_waker_will_wake_clone() {
struct NoopWaker;

impl Wake for NoopWaker {
fn wake(self: Arc<Self>) {}
}

let waker = Waker::from(Arc::new(NoopWaker));
let clone = waker.clone();

assert!(waker.will_wake(&clone));
assert!(clone.will_wake(&waker));
}

#[test]
fn test_local_waker_will_wake_clone() {
struct NoopWaker;

impl LocalWake for NoopWaker {
fn wake(self: Rc<Self>) {}
}

let waker = LocalWaker::from(Rc::new(NoopWaker));
let clone = waker.clone();

assert!(waker.will_wake(&clone));
assert!(clone.will_wake(&waker));
}

0 comments on commit 020de25

Please sign in to comment.