Replace some !Send resources with thread_local!

[joshua-holmes]

Objective

Work for issue #17682

What's in this PR:

• Removal of some !Send resources that Bevy uses internally
• Replaces !Send resources with thread_local! static

What this PR does not cover:

• The ability to create !Send resources still exists
• Tests that test !Send resources are present (and should not be removed until the ability to create !Send resources is removed)
• The example log_layers_ecs still uses a !Send resource. In this example, removing the !Send resource results in the system that uses it running on a thread other than the main thread, which doesn't work with lazily initialized thread_local! static data. Removing this !Send resource will need to be deferred until the System API is extended to support configuring which thread the System runs on. Once an issue for this work is created, it will be mentioned in 🐢 Eliminate !Send resources by supporting Send and !Send World in the same binary #17667

Once the System API is extended to allow control of which thread the System runs on, the rest of the !Send resources can be removed in a different PR.

[urben1680]

Won't this cause multiple worlds to share the same !Send resources?

[joshua-holmes]

Yes, in theory multiple worlds could share these resources. However, in practice in this PR, that is not happening.

Keep in mind, I am not changing the API in any way. I am only changing how a few internal variables are stored.

[joshua-holmes]

@alice-i-cecile, just pinging you here for review on GitHub as well, in case that's your preferred workflow

[chescock]

Looks good!

[alice-i-cecile]

Code quality is good, and I'm broadly on board with the direction, but I don't personally have the experience with these primitives required to verify correctness. @maniwani or @hymm , can I defer to you?

[hymm]

Changes are fairly straightforward. I think passing the EventLoop through a closure makes way more sense than smuggling it through a non send resource. My one worry is that nothing is forcing the gilrs systems to run on the main thread anymore other than the fact that wasm is single threaded. Maybe not too big of a deal though as it'll just panic with the thread local not being initialized and should be easy to fix if we do get wasm multithreading.

[aloucks]

gilrs uses mpsc::{Sender, Receiver} under the hood. From what I can tell, the Receiver is what causes Gilrs to be !Sync. I suspect that if the system is bouncing between threads and hitting different Gilrs instances (i.e. with different pairs of sender/receivers), you're going to have broken gamepad input. I haven't actually tested with a gamepad or Gilrs and I've come to this conclusion only by looking through the Gilrs code.

EDIT1: Was there some reason why you used thread_local! instead of Mutex<Gilrs> ?
EDIT2: I now see that it's also !Send in addition to !Sync on wasm32.

I think passing the EventLoop through a closure makes way more sense than smuggling it through a non send resource.

Agreed. I think this will help. However, the WinitWindows struct is still added via init_non_send_resource, which still prevents the App/World from being Send.

Allowing the App/World to be Send opens up some other options I'd like to explore with bevy_winit. I'm wondering if there's some design work opportunities for some kind of "lower level" systems that do not live inside the World and only executes on the "main" thread. These hypothetical type of systems would live along side the runner and the runner may be responsible for triggering them. I'll try to articulate this in more detail later this weekend.

[joshua-holmes]

@aloucks

Was there some reason why you used thread_local! instead of Mutex ?

I don't recall ever discussing that, so I'm going to say no. Looking at the gilrs code, I see your concern for using Receiver with thread_local! if we were using multiple threads with thread_local!. However, considering we are only using thread_local! with wasm here, I can't see a way that it would break anything.

However, the WinitWindows struct is still added via init_non_send_resource

You are correct. This PR does not completely get rid of !Send resources. We will need one more PR for that.

which still prevents the App/World from being Send.

Correct me if I'm wrong, but I believe the world is Send with the exception of !Send resources. We force the World to be Send here and just tell the user not to access !Send resources in a different thread, or else the program will panic.