add `InteriorMutableWorld` abstraction

[jakobhellermann]

alternative to #5922, implements #5956 builds on top of https://github.com/bevyengine/bevy/pull/6402

Objective

https://github.com/bevyengine/bevy/issues/5956 goes into more detail, but the TLDR is:

• bevy systems ensure disjoint accesses to resources and components, and for that to work there are methods World::get_resource_unchecked_mut(&self), ..., EntityRef::get_mut_unchecked(&self) etc.
• we don't have these unchecked methods for by_id variants, so third-party crate authors cannot build their own safe disjoint-access abstractions with these
• having _unchecked_mut methods is not great, because in their presence safe code can accidentally violate subtle invariants. Having to go through world.as_interior_mutable().unsafe_method() forces you to stop and think about what you want to write in your // SAFETY comment.

The alternative is to keep exposing _unchecked_mut variants for every operation that we want third-party crates to build upon, but we'd prefer to avoid using these methods alltogether: https://github.com/bevyengine/bevy/pull/5922#issuecomment-1241954543

Also, this is something that cannot be implemented outside of bevy, so having either this PR or #5922 as an escape hatch with lots of discouraging comments would be great.

Solution

• add InteriorMutableWorld with unsafe fn get_resource(&self), unsafe fn get_resource_mut(&self)
• add fn World::as_interior_mutable(&self) -> InteriorMutableWorld<'_>
• add InteriorMutableEntityRef with unsafe fn get, unsafe fn get_mut and the other utilities on EntityRef (no methods for spawning, despawning, insertion)
• use the InteriorMutableWorld abstraction in ReflectComponent, ReflectResource and ReflectAsset, so these APIs are easier to reason about
• remove World::get_resource_mut_unchecked, EntityRef::get_mut_unchecked and use unsafe { world.as_interior_mutable().get_mut() } and unsafe { world.as_interior_mutable().get_entity(entity)?.get_mut() } instead

This PR does not make use of InteriorMutableWorld for anywhere else in bevy_ecs such as SystemParam or Query. That is a much larger change, and I am convinced that having InteriorMutableWorld is already useful for third-party crates.

Implemented API:

struct World { .. }
impl World {
  fn as_interior_mutable(&self) -> InteriorMutableWorld<'_>;
}

struct InteriorMutableWorld<'w>(&'w World);
impl<'w> InteriorMutableWorld {
  unsafe fn world(&self) -> &World;

  fn get_entity(&self) -> InteriorMutableEntityRef<'w>; // returns 'w which is `'self` of the `World::as_interior_mutable(&'w self)`

  unsafe fn get_resource<T>(&self) -> Option<&'w T>;
  unsafe fn get_resource_by_id(&self, ComponentId) -> Option<&'w T>;
  unsafe fn get_resource_mut<T>(&self) -> Option<Mut<'w, T>>;
  unsafe fn get_resource_mut_by_id(&self) -> Option<MutUntyped<'w>>;
  unsafe fn get_non_send_resource<T>(&self) -> Option<&'w T>;
  unsafe fn get_non_send_resource_mut<T>(&self) -> Option<Mut<'w, T>>>;

  // not included: remove, remove_resource, despawn, anything that might change archetypes
}

struct InteriorMutableEntityRef<'w> { .. }
impl InteriorMutableEntityRef<'w> {
  unsafe fn get<T>(&self, Entity) -> Option<&'w T>;
  unsafe fn get_by_id(&self, Entity, ComponentId) -> Option<Ptr<'w>>;
  unsafe fn get_mut<T>(&self, Entity) -> Option<Mut<'w, T>>;
  unsafe fn get_mut_by_id(&self, Entity, ComponentId) -> Option<MutUntyped<'w>>;
  unsafe fn get_change_ticks<T>(&self, Entity) -> Option<Mut<'w, T>>;
  // fn id, archetype, contains, contains_id, containts_type_id
}

InteriorMutableWorld docs

Variant of the [World] where resource and component accesses takes a &World, and the responsibility to avoid aliasing violations are given to the caller instead of being checked at compile-time by rust's unique XOR shared rule.

Rationale

In rust, having a &mut World means that there are absolutely no other references to the safe world alive at the same time, without exceptions. Not even unsafe code can change this.

But there are situations where careful shared mutable access through a type is possible and safe. For this, rust provides the UnsafeCell escape hatch, which allows you to get a *mut T from a &UnsafeCell<T> and around which safe abstractions can be built.

Access to resources and components can be done uniquely using [World::resource_mut] and [World::entity_mut], and shared using [World::resource] and [World::entity]. These methods use lifetimes to check at compile time that no aliasing rules are being broken.

This alone is not enough to implement bevy systems where multiple systems can access disjoint parts of the world concurrently. For this, bevy stores all values of resources and components (and ComponentTicks) in UnsafeCells, and carefully validates disjoint access patterns using APIs like System::component_access.

A system then can be executed using System::run_unsafe with a &World and use methods with interior mutability to access resource values. access resource values.

Example Usage

[InteriorMutableWorld] can be used as a building block for writing APIs that safely allow disjoint access into the world. In the following example, the world is split into a resource access half and a component access half, where each one can safely hand out mutable references.

use bevy_ecs::world::World;
use bevy_ecs::change_detection::Mut;
use bevy_ecs::system::Resource;
use bevy_ecs::world::interior_mutable_world::InteriorMutableWorld;

// INVARIANT: existance of this struct means that users of it are the only ones being able to access resources in the world
struct OnlyResourceAccessWorld<'w>(InteriorMutableWorld<'w>);
// INVARIANT: existance of this struct means that users of it are the only ones being able to access components in the world
struct OnlyComponentAccessWorld<'w>(InteriorMutableWorld<'w>);

impl<'w> OnlyResourceAccessWorld<'w> {
    fn get_resource_mut<T: Resource>(&mut self) -> Option<Mut<'w, T>> {
        // SAFETY: resource access is allowed through this InteriorMutableWorld
        unsafe { self.0.get_resource_mut::<T>() }
    }
}
// impl<'w> OnlyComponentAccessWorld<'w> {
//     ...
// }

// the two interior mutable worlds borrow from the `&mut World`, so it cannot be accessed while they are live
fn split_world_access(world: &mut World) -> (OnlyResourceAccessWorld<'_>, OnlyComponentAccessWorld<'_>) {
    let resource_access = OnlyResourceAccessWorld(unsafe { world.as_interior_mutable() });
    let component_access = OnlyComponentAccessWorld(unsafe { world.as_interior_mutable() });
    (resource_access, component_access)
}

[jakobhellermann]

Good questions.

1. What external use cases do you see this being useful for?

The use case I had is for bevy-inspector-egui. There I want to get a mutable reference to one value (e.g. a component Handle<StandardMaterial>) pass store the remaining &mut World without that particular component in a context. Code walking a reflect object can then use the remaining context to look at the Assets<StandardMaterial> resource and get a &mut StandardMaterial. This recursively continues, so &mut StandardMaterial is then displayed but the context has a &mut World except for the handle component and the Assets<StandardMaterial> resource. I packaged this into the RestrictedWorldView abstraction. This basically builds a permission tree in the world:

&mut World
├── Handle@Entity1
└── everything -Handle@Entity1
    ├── Assets<StandardMaterial>
    └── everything -Handle -Assets<StandardMaterial>

Currently this stores a &World and works with _get_unchecked_mut methods and #5922, but a InteriorMutableWorld would document its purpose more clearly.

Another use case is running scripts in parallel (basically the same use case as bevy has with systems) where the signature

fn run_script(world: InteriorMutableWorld<'_>) {}

is much more self-documenting than the &World variant with unchecked methods.

2. In the long-term, how does this fit in with WorldCell

WorldCell is an API that contains a &mut World and hands out multiple &mut Rs to resources. It never actually uses &mut World methods, it only calls &world.get_resource_unchecked_mut internally. With InteriorMutableWorld it calls world.as_interior_mutable().get_resource_mut() instead and works exactly the same otherwise.

InteriorMutableWorld relates to WorldCell similar to how UnsafeCell relates to RefCell. It is used as an unsafe building block which needs additional structure and information to get a safe API on top of it.

3. In the long-term, what would you like to use this for internally?

In this PR: Change the signature of the unsafe methods in ReflectResource, ReflectComponent and ReflectAsset. Long-term: It could be used in System::run_unsafe, SystemParamFetch::get_param and QueryState::iter_unchecked & friends.

[BoxyUwU]

It needs to be &mut World -> InteriorMutableWorld<'_> otherwise this is relatively pointless. The goal behind InteriorMutableWorld should be to preserve the semantics that &World means shared/readonly, and &mut World means exclusive/mutable. We also dont want the unsafe methods for getting mutable borrows out of &World to have to care about what safe code might be doing with another &World which is only accomplished via &mut World -> InteriorMutableWorld<'_>

[jakobhellermann]

@BoxyUwU

It needs to be &mut World -> InteriorMutableWorld<'_> otherwise this is relatively pointless.

Right, the default can be a safe &mut World -> InteriorMutableWorld<'w> function which can be used most of the time.

We still need a function to go from &World to InteriorMutableWorld function, the question is who exactly has to uphold which invariants. Consider https://github.com/bevyengine/bevy/blob/688f13cd838efb1c0c44f2aa20ed844b45d6115f/crates/bevy_ecs/src/system/function_system.rs#L163-L174 In the future, get_unchecked_manual will take a InteriorMutableWorld. But get should take a &World, that is correct. What is the safety contract of get_unchecked_manual? Something like "You can only call this with a InteriorMutableWorld that may be used to access everything defined in this state's component_access_set".

And the new safety comment here would be

- `Param::Fetch` is `ReadOnlySystemParamFetch` so our component_access will be a set of only reads
- we have a `&World` which allows read access to everything
- so we can call `get_unchecked_manual` with an InteriorMutableWorld created from the World

Note that not the creation of the InteriorMutableWorld was unsafe, but passing that world to get_unchecked_manual.

---

A InteriorMutableWorld by default can be used for absolutely no resource/component access. I you have a

fn some_safe_function(world: InteriorMutableWorld<'_>) {
 // you can access *no* resources here, neither mutable nor immutable. What would you write as the Safety comment? You have no permission.
}

The way that the InteriorMutableWorld will be used is more like this:

/// SAFETY: may only be called with a interior mutable world that may be used to access MyResource mutably
unsafe fn some_unsafe_function(world: InteriorMutableWorld<'_>) {
  // SAFETY: we're allowed to access it
  let res = unsafe { world.get_resource_mut::<MyResource>() };
}

So what should be the safety comment of a function &World -> InteriorMutableWorld<'_> be? In my mental model, this is completely safe, because without additional context, you can do nothing with the InteriorMutableWorld. All its spicy methods are unsafe and need explicit permission through safety comments. Everyone function that accepts a InteriorMutableWorld needs to specify exactly what needs to be able to do with it, and every caller of such functions has to make sure that it can provide these guarantees. If you have a &mut World that's easy, and if you have a &World it is still possible sometimes (see SystemState::get where Param::Fetch: ReadyOnlySystemParamFetch).

[jakobhellermann]

&mut World -> InteriorMutableWorld creates a view into the world that may be used for anything. &World -> InteriorMutableWorld creates a view into the world that my be used for any read-only access.

Anything else (e.g. a world that can only access according to a FilteredAccessSet<ComponentId> or a world that may only access one specific resource) must be guaranteed by some additional requirements, such as where Param::Fetch: ReadOnlySystemParamFetch.

[jakobhellermann]

As another example,

https://github.com/bevyengine/bevy/blob/6763b31479246afddecbe4e717e138d4e9653dca/crates/bevy_ecs/src/query/fetch.rs#L333-L338

WorldQuery::init_fetch would take a InteriorMutableWorld that comes with no extra promises. As such, it can not be used to access any resources or components, but you can e.g. prepare a WriteFetch by storing the pointer to a particular ComponentSparseSet.

Doing this is completely safe, both inside the function (interior_mutable_world.storages().sparse_sets.get(component_id) is safe) and as the caller, since you can pass in any InteriorMutableWorld, even from a system with absolutely no access.

[BoxyUwU]

&World -> InteriorMutableWorld makes sense to me if we have a /// Can only be used for read only access on the function. My objection would be to allowing people to pull mutable references out of an InteriorMutableWorld made from an &World (which is what this PR currently does)

[jakobhellermann]

Agreed. What do you think of this API?

impl World {
  // Grants read-write access to the entire world
  fn as_interior_mutable(&mut self) -> InteriorMutableWorld<'_> {}
  // Grants read-only access to the entire world
  fn as_interior_mutable_readonly(&self) -> InteriorMutableWorld<'_> {}
}

?

[BoxyUwU]

Yeah that looks good to me

[DJMcNab]

Would something like PartialWorld be a better name?

[BoxyUwU]

I don't think so, it doesnt have anything making it seem like interior mutability is going on (InteriorMutableWorld is maybe a bit heavy handed lol but WorldCell is already taken... i guess UnsafeWorldCell would also work and kinda mirrors UnsafeCell's completely unchecked nature and mirroring of Cell..)

[jakobhellermann]

I added the methods

    /// Creates a new [`InteriorMutableWorld`] view with complete read+write access
    pub fn as_interior_mutable(&mut self) -> InteriorMutableWorld<'_>:
   
    /// Creates a new [`InteriorMutableWorld`] view only read access to everything
    pub fn as_interior_mutable_readonly(&self) -> InteriorMutableWorld<'_>:

    /// Creates a new [`InteriorMutableWorld`] with read+write access from a [&World](World).
    /// This is only a temporary measure until every `&World` that is semantically a [InteriorMutableWorld]
    /// has been replaced.
    pub(crate) fn as_interior_mutable_migration_internal(&self) -> InteriorMutableWorld<'_>;

The last one will be removed in a PR I will put up soon.

There is one new change here I'd like feedback on, regarding WorldCell: WorldCell previously contained a &mut World. It has &self methods that perform interior mutability. The obvious solution is to store a InteriorMutableWorld that gets created with full access in WorldCell::new(&mut World). This is great for all the methods, except the Drop impl which swaps the archetype_component_access field on the &mut World which is only used as a cache. This is obviously not possible through a &World/InteriorMutableWorld. My solution here was to put that field in an UnsafeCell that is only access in the Drop impl.

The alternative would be to keep storing &mut World and turning that into a InteriorMutableWorld just when we need it in get_resource etc., but that doesn't work because those methods take &self and as such can't use InteriorMutableWorld::new(&mut World).

[JoJoJet]

impl World {
  // Grants read-write access to the entire world
  fn as_interior_mutable(&mut self) -> InteriorMutableWorld<'_> {}
  // Grants read-only access to the entire world
  fn as_interior_mutable_readonly(&self) -> InteriorMutableWorld<'_> {}
}

What's the motivation for a read-only interior ~~mutable~~ world? Couldn't you just use &World for that?

[jakobhellermann]

What's the motivation for a read-only interior ~mutable~ world? Couldn't you just use &World for that?

If you have an unsafe method that takes a InteriorMutableWorld and want to call that from a function with access to a &World where you know that the unsafe method won't write to anything it is useful: https://github.com/bevyengine/bevy/pull/6972/commits/4f04a573740630e1da611d374d3dd4f770652cdb#diff-aa2678d5c3fe9adbe529e57567595039a862b26cc787cfea208678b4d323c2dbR172

[JoJoJet]

If the function doesn't write to anything, why not just have it take &World instead of InteriorMutableWorld?

Edit: Nevermind, I see. In the example you linked, get_unchecked_manual sometimes gives mutable access.

[jakobhellermann]

Because it can also be used by other methods with write access.

Stuff like this: https://github.com/bevyengine/bevy/blob/2938792c7d77ef2777909ac485b0042eb9fe634b/crates/bevy_ecs/src/query/state.rs#L1147

get_single_unchecked_manual is used by both get_single and get_single_mut.

[jakobhellermann]

rebased

[BoxyUwU]

Adding S-Controversial since this is a relatively large user facing unsafe abstraction with various different ways this could be done

[BoxyUwU]

Spent some time on reviewing this, generally quite happy with this and looking forward to propagating it all over bevy_ecs to be clearer around safety invariants and generally making it clearer wtf is going on :rofl: A few things I noticed that should be fixed:

• There is no InteriorMutableWorld::get_non_send_by_id/InteriorMutableWorld::get_non_send_mut_by_id
• All the InteriorMutableWorld methods seem to take &self instead of self, this probably isn't a big deal but there really isnt any reason to take &self and would introduce the possibility of accidentally writing an api like fn foo(&self) -> Foo<'_> instead of -> Foo<'w> :thinking: Generally I don't think the lifetime on the &self is meaningful so would be best to take self. same thing for InteriorMutableEntityRef.
• ReflectAsset::get_mut was removed, i'd want @MrGVSV to OK that since I am not familiar with the reflection stuff.
• EntityMut::get_mut is implemented by itself rather than calling into InteriorMutableEntityRef
• InteriorMutableEntityRef is missing a get_change_ticks_by_id method

[jakobhellermann]

Rebased and addressed comments.

EntityMut::get_mut is implemented by itself rather than calling into InteriorMutableEntityRef

Should every method be implemented in terms of InteriorMutableEntityRef if possible?

---

Currently, some methods returning a Ptr have a doc comment of

/// The returned pointer must not be used to modify the resource, and must not be /// dereferenced after the borrow of the [World] ends.

The first one is pretty obvious, you're not supposed to be able to mutate through Ptr. And the second one is not really applicable since we don't borrow the World. Should I just remove it or write something else?

[BoxyUwU]

Should every method be implemented in terms of InteriorMutableEntityRef if possible?

Yeah I think so, and same for InteriorMutableWorld. Generally having the impls of these be calling into &World methods feels a little sketchy since we have to be sure that the method doesnt treat the &World as shared access of the whole world (and same for EntityRef, although on top of that we might end up with EntityRef storing InteriorMutableEntityRef one day instead of &World...)

[james7132]

I haven't fully reviewed this yet, but a quick bikeshed on the name: UnsafeWorldCell? It behaves much more closely to UnsafeCell, which is what I've been treating &World as up until now. Also seems to have the same(-ish) relationship with WorldCell that UnsafeCell has with Cell.

[alice-i-cecile]

Once this is compiling + safety comment + renamed, I'd like to merge this ASAP. It's much easier to build safely change the ECS with this in place.

cc: @cart @BoxyUwU.

[jakobhellermann]

Okay, I rebased, added the safety comment and renamed InteriorMutableWorld -> UnsafeWorldCell InteriorMutableEntityRef -> UnsafeEntityRefCell as_interior_mutable -> as_unsafe_world_cell.

[alice-i-cecile]

Remember to update the PR description and title to match :)

[BoxyUwU]

I think that UnsafeWorldCell should have a way to pull &mut World out of it. Would be useful for the actual WorldCell impl and also the scheduler seems to want to go from &UnsafeCell<World> -> &mut World and it seems a bit sus to have the "most flexible unsafe world access" type be unable to express two of the interior mutability abstractions in bevy_ecs.

I think that we should add a unsafe fn(self) -> &'w mut World and make all the safe world metadata accesses unsafe with a safety invariant like "must be no active &mut World" :thinking:

[james7132]

I think that UnsafeWorldCell should have a way to pull &mut World out of it. Would be useful for the actual WorldCell impl and also the scheduler seems to want to go from &UnsafeCell<World> -> &mut World and it seems a bit sus to have the "most flexible unsafe world access" type be unable to express two of the interior mutability abstractions in bevy_ecs.

Isn't this unsound unless we put a &UnsafeCell<World> in it?

[BoxyUwU]

yes

[james7132]

Discussed with @BoxyUwU that their comments above are not blocking and can be done in a followup PR. Counting that as 2 SME approvals.

bors r+

[bors[bot]]

Pull request successfully merged into main.

Build succeeded:

• build-and-install-on-iOS
• build-android
• build (macos-latest)
• build (ubuntu-latest)
• build-wasm
• build (windows-latest)
• build-without-default-features (bevy)
• build-without-default-features (bevy_ecs)
• build-without-default-features (bevy_reflect)
• check-compiles
• check-doc
• check-missing-examples-in-docs
• ci
• markdownlint
• msrv
• run-examples
• run-examples-on-wasm
• run-examples-on-windows-dx12