Below is a definition for a struct that maintains an integer with a dumb operation on it: call into C++ to spawn a thread that will call back into Rust and increment the value, and in the meantime also increment it on this thread. The struct accepts the reference to itself as a mutable pinned reference (simulating e.g. the way Future::poll works). It hands off a const pointer to its embedded Mutex<u64> to C++, and when C++ calls back it converts that back into a shared reference to the Mutex<u64>.
use std::ffi::c_void;
use std::pin::Pin;
use std::sync::Mutex;
struct S {
val: Mutex<u64>,
_marker: std::marker::PhantomPinned,
}
impl S {
// Increment the value twice, in a really inefficient way and without
// waiting for both increments to take effect.
pub fn increment_twice(self: Pin<&mut Self>) {
let self_ref: &mut Self = unsafe { Pin::into_inner_unchecked(self) };
// Increment once in the background with C++.
unsafe {
start_thread_with_cpp(
cast_and_increment,
&self_ref.val as *const Mutex<u64> as *mut c_void,
);
}
// And another time here on this thread.
let mut val = self_ref.val.lock().unwrap();
*val += 1;
}
}
impl Drop for S {
fn drop(&mut self) {
// Assume that we somehow wait for all outstanding calls to finish here,
// i.e. there are no dangling pointers.
todo!();
}
}
unsafe extern "C" {
// Use C++ to spawn a thread that will eventually call the supplied function
// with the supplied argument.
fn start_thread_with_cpp(f: unsafe extern "C" fn(*mut c_void), p: *mut c_void);
}
// Cast back to *const and increment.
extern "C" fn cast_and_increment(p: *mut c_void) {
let val: &Mutex<u64> = unsafe { &*(p as *const Mutex<u64>) };
let mut val = val.lock().unwrap();
*val += 1;
}
As far as I'm aware, this code is more or less correct in the sense that it doesn't have any data races. The mutex does its own internal synchronization, so it's totally fine to access it concurrently from two threads.
But my question is: are there any aliasing violations here? We only ever create and hand off shared references to the mutex, but of course it's within the S and we do have a mutable reference to that struct. Does this count as a mutable reference to the mutex for the purposes of the aliasing rules?
Follow-up questions:
Where is this documented? I couldn't find any discussion of this in the places I looked.
Assuming this is a violation:
What real world problems could it cause, given that the mutex does its own internal synchronization?
How could we structure the code differently to avoid the aliasing violation if we are forced to accept a mutable reference to S (as in Future::poll)?
I could see how to do this with a separate allocation for the mutex that we only ever manage with a const pointer, but is there a way to do it without the extra overhead for the allocation and indirection?
Assuming this isn't a violation, how do we square that with the fact that for any other sort of field without internal synchronization we could clearly wind up with a problem if the callback read from the field and we wrote to it through the &mut Self in the original thread?
This looks to be a specific instance of a long-standing problem in Rust that others have encountered as well. Some examples of other places it has come up:
As you can see from the last two links, it seems that it's been known for years that much of the Rust async ecosystem relies on similar code that is technically unsound.
Maybe? Rust's aliasing rules are annoyingly but intentionally vaguely defined, so it sort of depends on who you ask:
The Rust Reference says:
&mut Tmust point to memory that is not read or written by any pointer not derived from the reference and that no other reference points to while they are live.
This pretty clearly says there is UB just because of the existence of the &mut S reference, particularly when combined with the section about pointed-to bytes:
The span of bytes a pointer or reference “points to” is determined by the pointer value and the size of the pointee type (using
size_of_val).
However the reference also acknowledges that "[t]he exact aliasing rules are not determined yet".
I believe the top contender for a formal model currently is Tree Borrows. It is pretty clear that this particular example is actually not undefined behavior just because of the existence of the &mut S—in contrast to the informal definition of the aliasing rules, under Tree Borrows the mere existence of a mutable reference isn't enough to trigger UB; you have to actually use it for something. Mutable references start in the Reserved state, and you can't reach the UB state without an access foreign to that reference followed by a local access.
However I believe this example would be able cause UB under Tree Borrows with two calls to increment_twice. That could allow interleaving of foreign and local accesses in a manner that takes the state machine for one reference to the UB state.
One easy one is that tools might reject it. UB detection tools like Miri or BorrowSanitizer might get upset about it (I haven't checked). Depending on your definition this may or may not count as a real world problem, since it would make it harder to use the tools to find other problems in a codebase that included this code.
More importantly but also more speculatively: it's possible that the compiler assuming memory isn't aliased could cause it to use that memory for temporary space in some optimization. For example it might spill registers there and then restore a legal value later. To be clear I haven't actually observed this. Perhaps I couldn't—it sounds like llvm's noalias attribute isn't consistently used by Rust to exploit the aliasing rules, despite their promise for opitmizations.
From the links above, it sounds like there is actually no way to do this correctly in stable Rust, and lots of code is technically unsound because of it. The leading proposal for fixing it seems to be UnsafePinned, which is described well by its RFC.