PhantomData for shared, temporarily-referenced pointer

1

I am trying to create a smart pointer type that is copiable and allows obtaining a temporary mutable reference to the pointee. For example:

use std::ptr::NonNull;
use std::marker::PhantomData;

#[derive(Clone, Copy)]
pub struct MyPointer<'a, T> {
    ptr: NonNull<T>,
    _phantom: PhantomData<&'a T>,
}

pub struct Foo { v: Vec<i32> }

impl Foo {
    pub fn with_callback(&mut self, f: impl for<'a> FnOnce(MyPointer<'a, Foo>)) {
        f(MyPointer { ptr: NonNull::from(&mut *self), _phantom: PhantomData });
    }
}

impl<'a> MyPointer<'a, Foo> {
    pub fn do_something_mut(&self, x: i32) {
        // SAFETY: All methods on `MyPointer` and `T` are non-reentrant,
        // and never create references lasting longer than the method body.
        let inner: &'a mut Foo = unsafe { &mut *self.ptr.as_ptr() };

        (*inner).v.push(x);
    }
}

Ideally, a user can safely write something like foo.with_callback(|x| { x.do_something_mut(1); x.do_something_mut(2); }).

My question is, what is the correct thing to put inside PhantomData? And just to double check, it shouldn't be necessary to use UnsafeCell?

2

Note: you may have to guard for reentrancy through the global allocator/panic_hook.
See: Who is responsible for preventing reentrancy issues through the allocator? · Issue #506 · rust-lang/unsafe-code-guidelines · GitHub

To answer your question, since MyPointer is acting like &Cell<T>. Put that in the PhantomData.

2 Likes
3

I don't think your playground is saying enough to give a great answer. But I'll point out some things.

As an example of not having enough information: what's the use of Foo::with_callback? It could just as easily take a FnOnce(&mut Foo) given the safety comment.

Speaking of the comment:

        // SAFETY: All methods on `MyPointer` and `T` are non-reentrant,
        // and never create references lasting longer than the method body.

  1. If this was in a generic method, how can you guarantee what methods T does or doesn't have?

  2. Even if T is always local, crates can define methods for your Ts via their own traits.

  3. Even if T is always local, and/or the comment defines something more strict than is actually needed, I'd consider making the construction of a MyPointer an unsafe operation. The comment is describing a restriction on things that don't happen locally, e.g. within the function.

&Cell<T> was a good answer, or &mut T would also work. You want T to be invariant, (like it is behind a &mut), or it will be unsound. It doesn't matter for your example, but it's unclear from your example if you're going to restrict the T to some known set of types or not.

You also need it to be not-Send and not-Sync, but NonNull<_> covers that as-is.

Not necessary if the NonNull<T> is always created from a &mut T.

You don't want to derive Copy and Clone, as that will require T: Copy and T: Clone. For example MyPointer<'_, Foo> isn't Copy or Clone in the playground (it would be Clone if Foo: Clone, but it's not possible for Foo to be Copy).

I don't think you want the 'a here:

        let inner: &'a mut Foo = unsafe { &mut *self.ptr.as_ptr() };

I don't know if there's a way for it to be exploited, but technically you could be asking for aliasing &mut to be created.

    let mut foo = Foo { v: Vec::new() };
    let mp = MyPointer { ptr: NonNull::from(&mut foo), _phantom: PhantomData };
    MyPointer::<'static, _>::do_something_mut(&mp, 0);
    MyPointer::<'static, _>::do_something_mut(&mp, 1);
3 Likes
4

Thank you both for the answers. The fields of MyPointer are private so users can't manually create an instance of it. (And when I say "all methods are non-reentrant", I guess I really mean "all methods that access self.ptr are non-reentrant").

The goal is really to run a closure with a handle to self providing some limited methods, and I want to guarantee that some cleanup code will be run before calling foo.with_callback() again (so returning a smart pointer and doing cleanup inside a Drop impl isn't safe since drop could be skipped with std::mem::forget()). I do plan to wrap the closure with std::panic::catch_unwind() to prevent re-entrancy from panicking. Thanks for the suggestion about alloc as well, though I don't know there's much I can reasonably do to guard against that for now (or custom panic handlers for that matter).

I would have used &'a mut T but I want the pointer to be copiable. I think &'a Cell<T> works.

Wouldn't 'a be covariant, preventing you from passing in a shorter lifetime to a method expecting &'static T?

5

I think you're assuming the lifetime of the PhantomData matches the lifetime of the &mut _ you created the NonNull<_> from, but there's nothing in your example ensuring that's the case (another reason to have a constructor IMO).

But even you make that true, so that the original &mut can't outlive the created MyPointer, you can still ask for aliasing &mut to be created by generalizing my 'static example.

Again, I don't know if that can be exploited if your methods are constrained enough ala your safety comment. But there's no reason to create a &'a mut _ specifically anyway, so why risk it.

6

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.