fix: add unwind safety to resource_scope

[joseph-gio]

Objective

The function World::resource_scope is a primitive that allows the user to get mutable access to a resource and the rest of the world at the same time. This is incredibly useful for exclusive systems, implementations of Command, and custom abstractions. It works by temporarily removing the resource from the world and running a user-provided closure with mutable access to both. After the closure completes, the resource is re-inserted into the world for other code to use.

With the current implementation, any panics originating in the user-provided closure will result in the resource being lost. This is problematic for a couple of reasons:

• Any users wishing to utilize std::panic::catch_unwind have to deal with the fact that the panic has left the world in an invalid state.
  • While some individuals consider catch_unwind to be bad practice, it is a native feature of Rust that we should provide support for. In certain situations it is quite valuable:
    • When creating apps targeting production, it is crucial to avoid hard-crashing on users, and in many cases it is not reasonable to ensure that every code path will always complete successfully. catch_unwind is useful for capturing panics and shifting into a controlled error state. This is hard to do when the world is left in an invalid state.
    • It is often useful to localize panics, rather than fully recover from them. When using threads or async tasks, the usual default is to localize panics to the original context that caused them. When implementing abstractions that give async tasks access to the world, it is useful to capture panics and propagate them back to the async context.
• Panics occurring inside of resource_scope can lead to knock-on errors, which can make debugging more difficult, as the user may need to sort through many errors to identify which one was the root cause.

The objective here is to make resource_scope panic-safe, ensuring that the resource is always restored to the world when unwinding from a panic (on platforms that support unwinding; panic=abort is unrecoverable by design). This enables more robust panic handling, and allows for more graceful shutdown sequences when panics are truly fatal.

Solution

Add a "guard" type that captures mutable access to the world and temporary ownership of the resource. This type has a Drop implementation that is responsible for re-inserting the resource at the end of the scope -- this pattern is similar to try ... finally blocks in many languages.

Testing

• Added a test that verifies the resource is properly restored when unwinding from resource_scope.
• Added a regression test for an existing edge-case triggered by World::clear_resources or World::clear_all.

[hymm]

Changing this to only panic in debug could be considered a breaking change. We might want to add a migration guide. I could go either way on this as this is a pretty minor change.

[hymm]

This feels weird to me. Why return None here as we already have the result? At the very least the documentation should be updated to say you can get None if f panicks. But given my comment above about the double panic in resource_scope I'm starting to believe that try_resource_scope might need to be changed to return a result instead to differentiate between when the resource doesn't exist vs when f has panicked.

[joseph-gio]

Yeah the behavior is a tad strange, but it's consistent with how it works on main if the resources get cleared. I'd be down to change that in a separate PR, but my intention here was to not change any behavior for the non-panicking case

[joseph-gio]

At the very least the documentation should be updated to say you can get None if f panicks

This isn't quite accurate -- if f panics you'll get !, rather than None. The function never completes or returns a value -- it just attempts to put the resource back into the world as it unwinds

[hymm]

Generally onboard with this change. Scoping this to debug_assertions means that any perf concerns are probably not worth worrying about. I do have a concern about how this changes the behavior of resource_scope though.

[ickshonpe]

This makes sense to me and I couldn't find any problems.