Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why's it seemingly okay to turn &mut T into &Cell<T> when self-references are involved, and then proceeding to change the value through a self-reference? #336

Closed
SoniEx2 opened this issue May 7, 2022 · 1 comment

Comments

@SoniEx2
Copy link

SoniEx2 commented May 7, 2022

This is related to #333 but a certain someone really likes blocking ppl and then continually interacting with said blocked ppl. We have this code (and yes this is as minimal as it gets, there are lots of moving parts here):

use std::cell::Cell;
use std::marker::PhantomPinned;
use std::pin::Pin;

use cell_project::cell_project;

struct Foo<'a> {
  x: Option<&'a Cell<Foo<'a>>>,
  y: usize,
}

impl<'a> Foo<'a> {
    // can a custom trait mirror the observable behaviour of Drop? (on nightly?)
    fn fake_drop(this: Pin<&'a Cell<Foo<'a>>>) {
        let bar = cell_project!(Foo<'_>, this.x);
        let inner = bar.get().unwrap();
        println!("{:p}", this.get_ref());
        println!("{:p}", inner);
        println!("{}", cell_project!(Foo<'_>, inner.y).get());
        cell_project!(Foo<'_>, inner.y).set(3);
    }
}

// not allowed to implement this
// impl<'a> Drop for Foo<'a> {
//     fn drop(&mut self) {
//     }
// }

// want: macro that does all this:

#[repr(transparent)]
struct FooOpaque {
  // unsafe field: not actually 'static
  inner: Foo<'static>,
  //_pinned: PhantomPinned,
}

impl FooOpaque {
    fn new(foo: impl for<'a> FnOnce(&'a ()) -> Foo<'a>) -> Self {
        #[allow(unreachable_code)]
        fn _prevent_problematic_drop(f: impl for<'x> Fn(&'x ()) -> Foo<'x>) {
            let _arg = ();
            let _foo: Foo<'_> = f(&_arg);
            let _cell = Cell::new(_foo);
            #[inline(never)]
            fn _f<'a>(_x: &'a Cell<Foo<'a>>) {}
            _f(&_cell);
        }
        Self {
            inner: unsafe { std::mem::transmute(foo(&())) },
            //_pinned: PhantomPinned,
        }
    }
    
    // this MUST NOT be a Cell because those have set(), replace(), swap(),
    // etc. but this here is just an example.
    fn operate_in<F, R>(pinned_cell: Pin<&Cell<Self>>, f: F) -> R
    where F: for<'a> FnOnce(Pin<&'a Cell<Foo<'a>>>) -> R {
        f(unsafe {
            pinned_cell.map_unchecked(|cell_ref| {
                std::mem::transmute(cell_project!(FooOpaque, cell_ref.inner))
            })
        })
    }
}

impl Drop for FooOpaque {
    fn drop(&mut self) {
        unsafe {
            // assume it was pinned.
            Self::operate_in(std::mem::transmute(self), |cell_ref| {
                Foo::fake_drop(cell_ref)
            });
        }
    }
}

fn now_can_move() {
    let foo = Box::pin(Cell::new(FooOpaque::new(|_| Foo {
        x: None,
        y: 0,
    })));
    FooOpaque::operate_in(foo.as_ref(), |foo| {
        let bar = cell_project!(Foo<'_>, foo.x);
        bar.set(Some(foo.get_ref()));
        println!("{}", cell_project!(Foo<'_>, foo.y).get());
        cell_project!(Foo<'_>, foo.y).set(1);
    });
    FooOpaque::operate_in(foo.as_ref(), |foo| {
        let bar = cell_project!(Foo<'_>, foo.x);
        let inner = bar.get().unwrap();
        println!("{:p}", foo.get_ref());
        println!("{:p}", inner);
        println!("{}", cell_project!(Foo<'_>, inner.y).get());
        cell_project!(Foo<'_>, inner.y).set(2);
    });
}

fn main() {
    now_can_move();
}

As-is, this is... potentially unsound, because FooOpaque needs to be !Unpin for soundness. However, miri doesn't catch any issues. Uncommenting the //_pinned: PhantomPinned, also changes nothing. So, seemingly, this doesn't behave like an async fn at all, despite being self-referential!

Have we solved async fn? Or, more likely, is this a hole in miri? (And if we have "solved" async fn, is it fine to publish a crate that does stuff like this?)

@RalfJung
Copy link
Member

However, miri doesn't catch any issues.

That is probably because Miri does not look into Pin, i.e., it does not treat Pin<&T>/Pin<&mut T>/Pin<Box<T>> special in any way. We do not currently recurse into fields for retagging. This is tracked in #125.

@RalfJung RalfJung closed this as not planned Won't fix, can't repro, duplicate, stale Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants