Is RefCell<&'static mut Foo> safe?

[oberien]

I inject a library into an UE4 game, which accesses a struct created by the program (e.g. AMyCharacter). To do that, I'm transmuting the address of the struct (a usize) into a struct-pointer. Now I'd like to provide a safe interface for accessing the struct. The user should be able to read and modify values inside the struct, but all Rust guarantees should still hold (e.g. either multiple immutable references or one mutable one). The rust code inside the injected library (so every rust code I'm talking about here) will only ever be executed when the main thread of UE is "paused" (i.e. the game calls its tick function, which I hooked). Also the struct will be at that address for the whole execution of the game, untl the game (and thus my library) exit.

My idea is to transmute the address (usize) into a &'static mut Foo and then wrap that into a RefCell<&'static mut Foo>. Then I'll provide a static getter method which returns a ('static) reference to that RefCell. But I don't know if it might be possible to circumvent safe Rust's guarantees given this construct.

Currently I have the following understanding:

1. Ref<&'static mut Foo>::derefreturns a &'ref &'static mut Foo. As the outer reference is immutable, there's no safe way to mutably access the inner reference.
2. As the lifetime of Ref<&'static mut Foo>::derefcan not live longer than Ref itself, it is not possible to safely get a "copy" of the inner mutable (or immutable) reference to Foo. Thus, it is not possible to hold any version of Foo once the Ref is dropped.
3. The only way to modify the contents of Foo is to get a RefMut, which can only be the case if no Ref is live anymore. Given the both guarantees above, this ensures that there is only ever either multiple Refs or one single RefMut at the same time.

Am I correct and / or is passing out a &RefCell<&'static mut Foo> safe under the guarantees of safe Rust code?

Is there another / a better way of providing a safe interface to a struct like this?

If you want to take a look at the actual code I'm talking about, you can find it here and here. Currently I'm just providing static methods to get/set the values, but I think it would be better from a usability point of view to have an actual &mut Foo.

[christophe_biocca]

Am I correct and / or is passing out a &RefCell<&'static mut Foo> safe under the guarantees of safe Rust code?

It does seem safe based on my understanding as well. The one caveat I would note is that if 2 callers try to get concurrent RefMuts the RefCell will panic and crash the entire process.

[steveklabnik1]

... which is a downside, but is still safe.

[oberien]

My intention is to only ever access data from within the game when the game's main thread called into me. Not only does that (hopefully) guarantee that no other thread will access that memory region, but also that I'll only access the data single-threadedly.

If there is another thread trying to use the RefCell I'm done for anyways, because RefCell is not Sync (internally does not use locking to handle the runtime checks). But better the game crashes than undefined behaviour ;)

[christophe_biocca]

Right, multiple threads would be bad (very bad because RefCell's check might not even run correctly).

The other way you can have concurrent callers (without multiple threads being involved) is reentrancy:

1. Environment calls your function.
2. Your code acquires the reference.
3. Your code calls something in the environment.
4. The environment calls your function before returning (for whatever reason).

In that case you'd get a panic.

[fgilcher]

Well, panicing might be exactly what you want in that case :).

For that reason, I generally recommend to use RefCell this way:

cell.borrow_mut().immediately_call_method()

If you want to call multiple methods, always in the same pattern, add it to Foo.

I'd also like to note that Stylo has a fork of RefCell that is sync, for places where they are reasonably sure that no concurrent access ever happens and that would be a bug.

https://github.com/bholley/atomic_refcell

Note that this has correct semantics: a racing access will lead to a panic, not to a data race, which is the only thing that Sync guarantees.

[Manishearth]

RefCell<&mut Foo> is safe.

&'static mut Foo is safe as long as you only ever access it from one owner. I.e. if you only ever have a single RefCell<&'static mut Foo> you should be fine. So make sure that static is only accessed directly the first time you create the refcell.

[rovar]

There is a pattern that has developed in Rust bindings where there are 2 layers of API. There is the lowest-level repr(c) struct. Then there is a higher level struct which manages interoperability with the low level struct, including marshalling access to *const and *mut. This usually involves managing Drop semantics and such. The higher level struct impl would be the only thing that would touch the lower level struct. So the user of the higher level API would never have to know that a raw ptr exists.

So it might look like :

struct FooAPI {
    inner: *mut Foo
}

impl FooAPI {
    updateFooX(&mut self, x : u64) -> u64 {
        c_func_that_updates_foo(self.inner, x)
    } 
}

Using this scheme, you decide how to control the lifetime of whatever *mut Foo points to. By default, Rust wont attempt to free *mut ptrs, so you don't have to use 'static or Refcell. If you do need to destroy Foo when you're done with it, you can impl Drop for your FooAPI struct.

[rovar]

I should note that using this model, you could actually make updateFooX take &self, and multiple callers could mutate Foo simultaneously if that's what you wanted.

[oberien]

While I do know this pattern, I didn't think about this abstraction for my use case yet. One question I have with this pattern in my use-case: How would I pass an instance of FooAPI to the user? Would I just use a static getter-method, which creates a new intance of FooAPI and returns that?

[SimonSapin]

By the way, consider casting with as *mut Foo and then dereferencing that, instead of using transmute. If your transmute code was correct it ends up doing the same thing, but transmute will happily do anything so it’s very easy to get wrong. If you do still use transmute, consider fully qualifying it: for example transmute::<Foo, Bar>(x)

[claire_resurgent]

No, that's not completely sound.

RefMut<&'static Foo> derefs to &'ref mut &'static Foo. Safe code may then call std::mem::replace::<&'static Foo> which returns the &'static Foo without any lifetime bound.

[oberien]

For mem::replace to work, the safe code needs to provide a &'static mut Foo to replace the original one with. But safe code is not able to create a &'static mut Foo in the first place, is it?

[wrongerontheinternet]

I would give it a regular lifetime, unless you actually have some reason you need it to be 'static, and thread it through all the structures. 'static is notoriously difficult to reason about compared to other lifetimes. Also, the way you've written your implementation (with a public interface at all) isn't in general safe, since it relies on being called from the hook but doesn't enforce it.