Replace some !Send resources with thread_local! (#17730)

joshua-holmes · web-flow · commit df6e136ab02a · 2025-03-04T07:48:02.000Z
# 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 #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.
diff --git a/crates/bevy_gilrs/src/gilrs_system.rs b/crates/bevy_gilrs/src/gilrs_system.rs
@@ -4,8 +4,6 @@ use crate::{
 };
 use bevy_ecs::event::EventWriter;
 use bevy_ecs::prelude::Commands;
-#[cfg(target_arch = "wasm32")]
-use bevy_ecs::system::NonSendMut;
 use bevy_ecs::system::ResMut;
 use bevy_input::gamepad::{
     GamepadConnection, GamepadConnectionEvent, RawGamepadAxisChangedEvent,
@@ -15,101 +13,103 @@ use gilrs::{ev::filter::axis_dpad_to_button, EventType, Filter};
 
 pub fn gilrs_event_startup_system(
     mut commands: Commands,
-    #[cfg(target_arch = "wasm32")] mut gilrs: NonSendMut<Gilrs>,
-    #[cfg(not(target_arch = "wasm32"))] mut gilrs: ResMut<Gilrs>,
+    mut gilrs: ResMut<Gilrs>,
     mut gamepads: ResMut<GilrsGamepads>,
     mut events: EventWriter<GamepadConnectionEvent>,
 ) {
-    for (id, gamepad) in gilrs.0.get().gamepads() {
-        // Create entity and add to mapping
-        let entity = commands.spawn_empty().id();
-        gamepads.id_to_entity.insert(id, entity);
-        gamepads.entity_to_id.insert(entity, id);
-
-        events.write(GamepadConnectionEvent {
-            gamepad: entity,
-            connection: GamepadConnection::Connected {
-                name: gamepad.name().to_string(),
-                vendor_id: gamepad.vendor_id(),
-                product_id: gamepad.product_id(),
-            },
-        });
-    }
+    gilrs.with(|gilrs| {
+        for (id, gamepad) in gilrs.gamepads() {
+            // Create entity and add to mapping
+            let entity = commands.spawn_empty().id();
+            gamepads.id_to_entity.insert(id, entity);
+            gamepads.entity_to_id.insert(entity, id);
+            events.write(GamepadConnectionEvent {
+                gamepad: entity,
+                connection: GamepadConnection::Connected {
+                    name: gamepad.name().to_string(),
+                    vendor_id: gamepad.vendor_id(),
+                    product_id: gamepad.product_id(),
+                },
+            });
+        }
+    });
 }
 
 pub fn gilrs_event_system(
     mut commands: Commands,
-    #[cfg(target_arch = "wasm32")] mut gilrs: NonSendMut<Gilrs>,
-    #[cfg(not(target_arch = "wasm32"))] mut gilrs: ResMut<Gilrs>,
+    mut gilrs: ResMut<Gilrs>,
     mut gamepads: ResMut<GilrsGamepads>,
     mut events: EventWriter<RawGamepadEvent>,
     mut connection_events: EventWriter<GamepadConnectionEvent>,
     mut button_events: EventWriter<RawGamepadButtonChangedEvent>,
     mut axis_event: EventWriter<RawGamepadAxisChangedEvent>,
 ) {
-    let gilrs = gilrs.0.get();
-    while let Some(gilrs_event) = gilrs.next_event().filter_ev(&axis_dpad_to_button, gilrs) {
-        gilrs.update(&gilrs_event);
-        match gilrs_event.event {
-            EventType::Connected => {
-                let pad = gilrs.gamepad(gilrs_event.id);
-                let entity = gamepads.get_entity(gilrs_event.id).unwrap_or_else(|| {
-                    let entity = commands.spawn_empty().id();
-                    gamepads.id_to_entity.insert(gilrs_event.id, entity);
-                    gamepads.entity_to_id.insert(entity, gilrs_event.id);
-                    entity
-                });
-
-                let event = GamepadConnectionEvent::new(
-                    entity,
-                    GamepadConnection::Connected {
-                        name: pad.name().to_string(),
-                        vendor_id: pad.vendor_id(),
-                        product_id: pad.product_id(),
-                    },
-                );
+    gilrs.with(|gilrs| {
+        while let Some(gilrs_event) = gilrs.next_event().filter_ev(&axis_dpad_to_button, gilrs) {
+            gilrs.update(&gilrs_event);
+            match gilrs_event.event {
+                EventType::Connected => {
+                    let pad = gilrs.gamepad(gilrs_event.id);
+                    let entity = gamepads.get_entity(gilrs_event.id).unwrap_or_else(|| {
+                        let entity = commands.spawn_empty().id();
+                        gamepads.id_to_entity.insert(gilrs_event.id, entity);
+                        gamepads.entity_to_id.insert(entity, gilrs_event.id);
+                        entity
+                    });
 
-                events.write(event.clone().into());
-                connection_events.write(event);
-            }
-            EventType::Disconnected => {
-                let gamepad = gamepads
-                    .id_to_entity
-                    .get(&gilrs_event.id)
-                    .copied()
-                    .expect("mapping should exist from connection");
-                let event = GamepadConnectionEvent::new(gamepad, GamepadConnection::Disconnected);
-                events.write(event.clone().into());
-                connection_events.write(event);
-            }
-            EventType::ButtonChanged(gilrs_button, raw_value, _) => {
-                let Some(button) = convert_button(gilrs_button) else {
-                    continue;
-                };
-                let gamepad = gamepads
-                    .id_to_entity
-                    .get(&gilrs_event.id)
-                    .copied()
-                    .expect("mapping should exist from connection");
-                events.write(RawGamepadButtonChangedEvent::new(gamepad, button, raw_value).into());
-                button_events.write(RawGamepadButtonChangedEvent::new(
-                    gamepad, button, raw_value,
-                ));
-            }
-            EventType::AxisChanged(gilrs_axis, raw_value, _) => {
-                let Some(axis) = convert_axis(gilrs_axis) else {
-                    continue;
-                };
-                let gamepad = gamepads
-                    .id_to_entity
-                    .get(&gilrs_event.id)
-                    .copied()
-                    .expect("mapping should exist from connection");
-                events.write(RawGamepadAxisChangedEvent::new(gamepad, axis, raw_value).into());
-                axis_event.write(RawGamepadAxisChangedEvent::new(gamepad, axis, raw_value));
-            }
-            _ => (),
-        };
-    }
-    gilrs.inc();
+                    let event = GamepadConnectionEvent::new(
+                        entity,
+                        GamepadConnection::Connected {
+                            name: pad.name().to_string(),
+                            vendor_id: pad.vendor_id(),
+                            product_id: pad.product_id(),
+                        },
+                    );
+                    events.write(event.clone().into());
+                    connection_events.write(event);
+                }
+                EventType::Disconnected => {
+                    let gamepad = gamepads
+                        .id_to_entity
+                        .get(&gilrs_event.id)
+                        .copied()
+                        .expect("mapping should exist from connection");
+                    let event =
+                        GamepadConnectionEvent::new(gamepad, GamepadConnection::Disconnected);
+                    events.write(event.clone().into());
+                    connection_events.write(event);
+                }
+                EventType::ButtonChanged(gilrs_button, raw_value, _) => {
+                    let Some(button) = convert_button(gilrs_button) else {
+                        continue;
+                    };
+                    let gamepad = gamepads
+                        .id_to_entity
+                        .get(&gilrs_event.id)
+                        .copied()
+                        .expect("mapping should exist from connection");
+                    events.write(
+                        RawGamepadButtonChangedEvent::new(gamepad, button, raw_value).into(),
+                    );
+                    button_events.write(RawGamepadButtonChangedEvent::new(
+                        gamepad, button, raw_value,
+                    ));
+                }
+                EventType::AxisChanged(gilrs_axis, raw_value, _) => {
+                    let Some(axis) = convert_axis(gilrs_axis) else {
+                        continue;
+                    };
+                    let gamepad = gamepads
+                        .id_to_entity
+                        .get(&gilrs_event.id)
+                        .copied()
+                        .expect("mapping should exist from connection");
+                    events.write(RawGamepadAxisChangedEvent::new(gamepad, axis, raw_value).into());
+                    axis_event.write(RawGamepadAxisChangedEvent::new(gamepad, axis, raw_value));
+                }
+                _ => (),
+            };
+        }
+        gilrs.inc();
+    });
 }
diff --git a/crates/bevy_gilrs/src/lib.rs b/crates/bevy_gilrs/src/lib.rs
@@ -14,19 +14,48 @@ mod converter;
 mod gilrs_system;
 mod rumble;
 
+#[cfg(not(target_arch = "wasm32"))]
+use bevy_utils::synccell::SyncCell;
+
+#[cfg(target_arch = "wasm32")]
+use core::cell::RefCell;
+
 use bevy_app::{App, Plugin, PostUpdate, PreStartup, PreUpdate};
 use bevy_ecs::entity::hash_map::EntityHashMap;
 use bevy_ecs::prelude::*;
 use bevy_input::InputSystem;
 use bevy_platform_support::collections::HashMap;
-use bevy_utils::synccell::SyncCell;
 use gilrs::GilrsBuilder;
 use gilrs_system::{gilrs_event_startup_system, gilrs_event_system};
 use rumble::{play_gilrs_rumble, RunningRumbleEffects};
 use tracing::error;
 
-#[cfg_attr(not(target_arch = "wasm32"), derive(Resource))]
-pub(crate) struct Gilrs(pub SyncCell<gilrs::Gilrs>);
+#[cfg(target_arch = "wasm32")]
+thread_local! {
+    /// Temporary storage of gilrs data to replace usage of `!Send` resources. This will be replaced with proper
+    /// storage of `!Send` data after issue #17667 is complete.
+    ///
+    /// Using a `thread_local!` here relies on the fact that wasm32 can only be single threaded. Previously, we used a
+    /// `NonSendMut` parameter, which told Bevy that the system was `!Send`, but now with the removal of `!Send`
+    /// resource/system parameter usage, there is no internal guarantee that the system will run in only one thread, so
+    /// we need to rely on the platform to make such a guarantee.
+    static GILRS: RefCell<Option<gilrs::Gilrs>> = const { RefCell::new(None) };
+}
+
+#[derive(Resource)]
+pub(crate) struct Gilrs {
+    #[cfg(not(target_arch = "wasm32"))]
+    cell: SyncCell<gilrs::Gilrs>,
+}
+impl Gilrs {
+    #[inline]
+    pub fn with(&mut self, f: impl FnOnce(&mut gilrs::Gilrs)) {
+        #[cfg(target_arch = "wasm32")]
+        GILRS.with(|g| f(g.borrow_mut().as_mut().expect("GILRS was not initialized")));
+        #[cfg(not(target_arch = "wasm32"))]
+        f(self.cell.get());
+    }
+}
 
 /// A [`resource`](Resource) with the mapping of connected [`gilrs::GamepadId`] and their [`Entity`].
 #[derive(Debug, Default, Resource)]
@@ -65,10 +94,15 @@ impl Plugin for GilrsPlugin {
             .build()
         {
             Ok(gilrs) => {
+                let g = Gilrs {
+                    #[cfg(not(target_arch = "wasm32"))]
+                    cell: SyncCell::new(gilrs),
+                };
                 #[cfg(target_arch = "wasm32")]
-                app.insert_non_send_resource(Gilrs(SyncCell::new(gilrs)));
-                #[cfg(not(target_arch = "wasm32"))]
-                app.insert_resource(Gilrs(SyncCell::new(gilrs)));
+                GILRS.with(|g| {
+                    g.replace(Some(gilrs));
+                });
+                app.insert_resource(g);
                 app.init_resource::<GilrsGamepads>();
                 app.init_resource::<RunningRumbleEffects>()
                     .add_systems(PreStartup, gilrs_event_startup_system)
diff --git a/crates/bevy_gilrs/src/rumble.rs b/crates/bevy_gilrs/src/rumble.rs
@@ -1,8 +1,6 @@
 //! Handle user specified rumble request events.
 use crate::{Gilrs, GilrsGamepads};
 use bevy_ecs::prelude::{EventReader, Res, ResMut, Resource};
-#[cfg(target_arch = "wasm32")]
-use bevy_ecs::system::NonSendMut;
 use bevy_input::gamepad::{GamepadRumbleIntensity, GamepadRumbleRequest};
 use bevy_platform_support::collections::HashMap;
 use bevy_time::{Real, Time};
@@ -128,42 +126,42 @@ fn handle_rumble_request(
 }
 pub(crate) fn play_gilrs_rumble(
     time: Res<Time<Real>>,
-    #[cfg(target_arch = "wasm32")] mut gilrs: NonSendMut<Gilrs>,
-    #[cfg(not(target_arch = "wasm32"))] mut gilrs: ResMut<Gilrs>,
+    mut gilrs: ResMut<Gilrs>,
     gamepads: Res<GilrsGamepads>,
     mut requests: EventReader<GamepadRumbleRequest>,
     mut running_rumbles: ResMut<RunningRumbleEffects>,
 ) {
-    let gilrs = gilrs.0.get();
-    let current_time = time.elapsed();
-    // Remove outdated rumble effects.
-    for rumbles in running_rumbles.rumbles.values_mut() {
-        // `ff::Effect` uses RAII, dropping = deactivating
-        rumbles.retain(|RunningRumble { deadline, .. }| *deadline >= current_time);
-    }
-    running_rumbles
-        .rumbles
-        .retain(|_gamepad, rumbles| !rumbles.is_empty());
-
-    // Add new effects.
-    for rumble in requests.read().cloned() {
-        let gamepad = rumble.gamepad();
-        match handle_rumble_request(&mut running_rumbles, gilrs, &gamepads, rumble, current_time) {
-            Ok(()) => {}
-            Err(RumbleError::GilrsError(err)) => {
-                if let ff::Error::FfNotSupported(_) = err {
-                    debug!("Tried to rumble {gamepad:?}, but it doesn't support force feedback");
-                } else {
-                    warn!(
-                    "Tried to handle rumble request for {gamepad:?} but an error occurred: {err}"
-                    );
+    gilrs.with(|gilrs| {
+        let current_time = time.elapsed();
+        // Remove outdated rumble effects.
+        for rumbles in running_rumbles.rumbles.values_mut() {
+            // `ff::Effect` uses RAII, dropping = deactivating
+            rumbles.retain(|RunningRumble { deadline, .. }| *deadline >= current_time);
+        }
+        running_rumbles
+            .rumbles
+            .retain(|_gamepad, rumbles| !rumbles.is_empty());
+
+        // Add new effects.
+        for rumble in requests.read().cloned() {
+            let gamepad = rumble.gamepad();
+            match handle_rumble_request(&mut running_rumbles, gilrs, &gamepads, rumble, current_time) {
+                Ok(()) => {}
+                Err(RumbleError::GilrsError(err)) => {
+                    if let ff::Error::FfNotSupported(_) = err {
+                        debug!("Tried to rumble {gamepad:?}, but it doesn't support force feedback");
+                    } else {
+                        warn!(
+                        "Tried to handle rumble request for {gamepad:?} but an error occurred: {err}"
+                        );
+                    }
                 }
-            }
-            Err(RumbleError::GamepadNotFound) => {
-                warn!("Tried to handle rumble request {gamepad:?} but it doesn't exist!");
-            }
-        };
-    }
+                Err(RumbleError::GamepadNotFound) => {
+                    warn!("Tried to handle rumble request {gamepad:?} but it doesn't exist!");
+                }
+            };
+        }
+    });
 }
 
 #[cfg(test)]
diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs
@@ -120,11 +120,15 @@ impl<T: Event> Plugin for WinitPlugin<T> {
             event_loop_builder.with_android_app(bevy_window::ANDROID_APP.get().expect(msg).clone());
         }
 
+        let event_loop = event_loop_builder
+            .build()
+            .expect("Failed to build event loop");
+
         app.init_non_send_resource::<WinitWindows>()
             .init_resource::<WinitMonitors>()
             .init_resource::<WinitSettings>()
             .add_event::<RawWinitWindowEvent>()
-            .set_runner(winit_runner::<T>)
+            .set_runner(|app| winit_runner(app, event_loop))
             .add_systems(
                 Last,
                 (
@@ -139,14 +143,6 @@ impl<T: Event> Plugin for WinitPlugin<T> {
 
         app.add_plugins(AccessKitPlugin);
         app.add_plugins(cursor::CursorPlugin);
-
-        let event_loop = event_loop_builder
-            .build()
-            .expect("Failed to build event loop");
-
-        // `winit`'s windows are bound to the event loop that created them, so the event loop must
-        // be inserted as a resource here to pass it onto the runner.
-        app.insert_non_send_resource(event_loop);
     }
 }
 
diff --git a/crates/bevy_winit/src/state.rs b/crates/bevy_winit/src/state.rs
