Let there be a struct for which an user may provide a callback. This callback is sent to another thread, which passes the self: Pin<&Self> to the call by converting a raw pointer.
The value lives on the stack and therefore we must clean up the callback on destruction. We implement this on Drop, and protect everything with mutexes so that we wait if the callback is running.
The problem: when running Drop, we have a &mut self reference. While we wait for the callback to complete (via mutex), we have both a &mut self (on the drop function, currently waiting for the mutex) and a &self reference (on the callback, currently holding the mutex) to the same object, therefore UB as far as I know.
Is it possible to handle this without UB? If the value must live on the heap (accessed via a pointer) it seems solvable (as the destructor can only be called when there are no other references), but it would be nicer if this could be solved without resorting to allocation.
Pseudocode:
struct Foo {
// This object originates from a C library and has callbacks
// which are called on another thread.
inner: FfiObject,
// Needed in order to correctly destroy the callback.
callback: UnsafeCell<Option<Box<dyn Any>>>,
...
}
impl Foo {
pub fn set_callback<F>(self: Pin<&Self>, mut c: F)
where
F: FnMut(Pin<&Self>) + Send + Sync,
{
let ptr = self.get_ref() as *const Self;
let c = Box::new(|| {
// When callback is called, the mutex contained by inner is locked
// by the C library
c(unsafe { Pin::new_unchecked(&*ptr) })
};
unsafe {
inner.lock_mutex();
inner.set_callback(c);
*self.callback.get() = Some(c);
inner.unlock_mutex();
}
}
}
impl Drop for Foo {
fn drop(&mut self) {
unsafe {
inner.lock_mutex(); // Problem here!
// If the callback is running on another thread,
// the mutex blocks. However, this thread has
// as &mut self reference and the other
// thread has a self: Pin<&Self> reference to
// the same object.
inner.destroy(); // This guarantees the callback is no longer called
inner.unlock_mutex();
}
}
}
I'm having trouble understanding what you mean from your description. Is the callback contained in the struct, or does it operate on the struct? Could you sketch out the general pattern here (who's calling what with what arguments) with some quick pseudocode?
I don't think I understand this design very well, but the thread that is running the callback is synthesizing a reference to your object. The fact that it can do that while you are dropping the object seems like a problem. At some point you are converting a raw pointer to a reference, but the lifetime of the resulting reference should be tied to your original object in some way.
This kind of self-referential structure is hard to reason about.. I'm not 100% sure on the safety of the following code, but can you pull out the callback into a containing struct instead, something like this?
EDIT: I'm actually not sure if a &mut Foo would still invalidate any pointer to the callback or the FooInner since &mut UnsafeCell<T> is essentially the same as &mut T..
I can't think of any way to avoid this without heap allocation, so if you can afford to make another allocation, I would just only store pointers in your struct:
struct FooInner {
inner: FfiObject,
}
struct Foo {
callback: Option<NonNull<dyn Any>>,
inner: NonNull<FooInner>,
}
impl Foo {
pub fn set_callback<F>(self: Pin<&Self>, mut c: F)
where
F: FnMut(Pin<&FooInner>) + Send + Sync,
{
let ptr = self.inner;
let c = Box::new(|| {
c(unsafe { Pin::new_unchecked(ptr.as_ref()) })
};
unsafe {
let inner = self.inner.as_ref();
inner.lock_mutex();
inner.set_callback(c);
self.callback = NonNull::new(c as *mut _);
inner.unlock_mutex();
}
}
}
impl Drop for Foo {
fn drop(&mut self) {
unsafe {
let inner = self.inner.as_ref();
inner.lock_mutex();
inner.destroy();
inner.unlock_mutex();
}
unsafe {
drop(Box::from_raw(self.inner.as_ptr()));
if let Some(callback) = NonNull::new(self.callback) {
drop(Box::from_raw(self.callback.as_ptr()));
}
}
}
}
This is the core open issue with pinning. The short version is that there's no 100% surely guaranteed way to allow aliasing like this currently.
A shortened version of the long version is that unimplementing Unpin (by containing PhantomPinned) disables the noalias of &mut T to enable the use of aliasing in this manner.
Because Rust doesn't have a formal definition of the runtime aliasing rules, this is mostly just "what works today" kind of observation. If you utilize unindirected aliasing in this manner, you're taking on some risk that the rules might change in the future and require you to adjust your code to adapt to the new rules (e.g. wrap it in some sort of UnsafeCellMut).
Right. With allocations this is very possible indeed, and doesn't need pinning either. Also by using weak references this can be done in safer manner.
So this seems unsolvable at least if one doesn't want to rely on UB that works for now.
This problem seems related to this as well: intrusive.md. I could not find much more about this specific kind of issue. If anyone can provide search terms or posts where this is discussed, I'd appreciate lots.
at least if one doesn't want to rely on UB that works for now.