WorldPointer is unsound

[jrobsonchase]

Like, really really unsound.

Paraphrasing dtonlnay, an API is unsound if it's possible to break its intended invariants in safe code.

WorldPointer's stated invariant is:

This pointer does not prevent dangling pointers, i.e. you must ensure the world is not dropped while any world pointers still exist, the world must also not change, from the moment a world pointer is created it must always point to the same world.

This isn't entirely correct, and is also incomplete and trivially broken in safe third-party code. The "you must ensure the world is not dropped while any world pointers still exist" bit implies that it's the pointed-to World that can't be dropped, while it's actually the mutable reference itself that determines the maximum lifetime of a WorldPointer. Where the World lives is entirely the purview of the bevy internals, and it may be moved around without warning, thus invalidating any previous references to it. Additionally, since the WorldPointer creates a second, sharable mutable reference to the World, it also needs to include the invariant that the original &mut World may only be used through the WorldPointer while it's in scope. Otherwise, it would violate Rust's aliasing rules.

The first invariant, that the WorldPointer must not outlive the &mut World, is broken as soon as ownership gets passed to another function. Since it's 'static, nothing stops other functions from squirreling it away somewhere that will outlive the &mut World. Since it's also Clone, this isn't even preventable by only passing around reference to it, since callees can simply .clone() it to get an owned version of it, and then do with that whatever they will. The clone method should probably be changed to be an unsafe struct method rather than the safe trait method that it currently is. Then, if only references to the WorldPointer are handed out, the onus is on the callees of the unsafe clone method to ensure that the invariants are upheld.

The second invariant, that the original &mut World must only be used through the WorldPointer is, for the most part, only the concern of the original creator of the pointer, since they're the only one who ever has access to the original &mut World to violate it. So long as the first invariant is upheld, it should be possible to guarantee that the second is also upheld entirely within the scope that the WorldPointer was created.

As far as current invalid usage goes, I believe that any APIProvider that stores the world in a script context (both the rhai and lua BevyAPIProviders for instance) technically violate this, since there's no way for these providers to "un-setup" the script runtime, and destroy their stored WorldPointer before the ScriptHost's handle_events method returns and invalidates the &mut World.

[makspll]

As discussed on discord, yes this is correct, the WorldPointer is unsound, but this only comes into play when storing away the struct and trying to use it in another frame, the standard APIProviders refresh this struct every time before handling events, so unless scripts keep a hold of that somewhere, it's all fine. Thanks for the PR!