Box world to prevent pointer invalidation

[radiantgurl]

Objective

• During subapp insertions, and depending on the hashmap implementation, subapps may be moved around for a rehash and reallocation. If there are any pointers held to the world unsafely, such as by the rendering pipeline, modifying subapps will invalidate them.

Solution

• Boxing the world would pin it in place preventing the world pointer from becoming invalid if the subapps change.

Testing

• Untested, i wrote the changes from my phone.

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[PhantomMorrigan]

This should use Pin<World> instead, while I don't think box would ever reallocate the world here, it's not guaranteed.Pin exists specifically for this kind of thing.

[SludgePhD]

Why do you need to hold on to a raw pointer to World? That pointer will get invalidated when a &mut World is created by any plugin or user code, whether the World is boxed or not.

If bevy does decide to guarantee address stability for things like this, that guarantee should also be explicitly communicated in the documentation of the relevant types.

[SludgePhD]

This should use Pin<World> instead, while I don't think box would ever reallocate the world here, it's not guaranteed.Pin exists specifically for this kind of thing.

Pin<Box<World>> does not prevent destructing and deallocating its contents. It is generally fine to use Box to achieve address stability, but the code that does so has to precisely define what invariants it actually provides. App allows callers to remove SubApps, and it allows obtaining mutable references to them (and their Worlds), which invalidates all other pointers.

[PhantomMorrigan]

Oh, also this requires World to not implement Unpin. Which it does, because bevy currently gives absolutely no address guarantees. I think it's probably more reasonable to pin the whole SubApp instead of just the World.

As for the why, the answer is modding, and being able to do stuff like modifying SubApps during runtime. It's a bit of a weird exotic use case, but it is pretty interesting.

[james7132]

I don't recall us having any point where we're holding non-lifetimed pointers into the render world ,and more specifically into it's on-stack representation. @radiantgurl could you illustrate where this is happening or what use case you envision this to enable? It's otherwise unclear what the motivation here is.

[radiantgurl]

Superseded by #20975

This PR was supoosed to add constraints ensuring certain safety properties, but i found a better way of fixing the problem.