Skip to content

Remove remaining internal use of !Send resources #18386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
May 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
899926d
Remove `WinitWindows` non-send res
joshua-holmes Mar 18, 2025
707bc1e
Replace `AccessKitAdapter` non-send resource
joshua-holmes Mar 19, 2025
dc1b015
Remove unnecessary `NonSendMarker`s
joshua-holmes Mar 19, 2025
e67d675
Expose `visible` to scope outside of `.with()`
joshua-holmes Mar 19, 2025
3f014d1
Simplify the use of `.with()`
joshua-holmes Mar 19, 2025
bfa24cc
Destructure `windows`
joshua-holmes Mar 19, 2025
bcc56ac
Remove unnecessary mutability
joshua-holmes Mar 20, 2025
aa14237
Remove unnecessary `NonSendMarker`
joshua-holmes Mar 20, 2025
7bce0d2
Use `::default()` when initializing `WINIT_WINDOWS` and `ACCESS_KIT_A…
joshua-holmes Mar 20, 2025
f6f681e
Import `NonSendMarker`
joshua-holmes Mar 24, 2025
beda4ac
Merge branch 'main' into no-more-non-send
joshua-holmes Mar 29, 2025
9c02770
Merge branch 'main' into no-more-non-send
joshua-holmes Apr 2, 2025
7688ad5
Add regression tests to ensure `World` is truely `Send`
joshua-holmes Apr 10, 2025
8e07dcd
Don't run `world_is_truly_send` test on macos
joshua-holmes Apr 10, 2025
b908936
Add `DISPLAY` env var for linux tests
joshua-holmes Apr 10, 2025
3e98571
Remove previously added env vars + `world_is_truly_send` test in `bev…
joshua-holmes Apr 10, 2025
8907bf1
Merge remote-tracking branch 'upstream' into no-more-non-send
joshua-holmes May 1, 2025
3838f85
Fix Android compilation bugs
joshua-holmes May 6, 2025
ac162fb
remove options from thread locals
mockersf May 6, 2025
955fe3e
fix compilation on android
mockersf May 6, 2025
b8d7c67
manual run of systems outside of borrowed thread locals
mockersf May 6, 2025
0b84ea8
expect, not allow
mockersf May 6, 2025
2aacd73
cfg_attr the expect
mockersf May 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions crates/bevy_gilrs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,22 @@ impl Plugin for GilrsPlugin {
}
}
}

#[cfg(test)]
mod tests {
use super::*;

// Regression test for https://github.com/bevyengine/bevy/issues/17697
#[test]
fn world_is_truly_send() {
let mut app = App::new();
app.add_plugins(GilrsPlugin);
let world = core::mem::take(app.world_mut());

let handler = std::thread::spawn(move || {
drop(world);
});

handler.join().unwrap();
}
}
87 changes: 52 additions & 35 deletions crates/bevy_winit/src/accessibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use alloc::{collections::VecDeque, sync::Arc};
use bevy_input_focus::InputFocus;
use core::cell::RefCell;
use std::sync::Mutex;
use winit::event_loop::ActiveEventLoop;

Expand All @@ -16,13 +17,26 @@ use bevy_a11y::{
};
use bevy_app::{App, Plugin, PostUpdate};
use bevy_derive::{Deref, DerefMut};
use bevy_ecs::{entity::EntityHashMap, prelude::*};
use bevy_ecs::{entity::EntityHashMap, prelude::*, system::NonSendMarker};
use bevy_window::{PrimaryWindow, Window, WindowClosed};

thread_local! {
/// Temporary storage of access kit adapter data to replace usage of `!Send` resources. This will be replaced with proper
/// storage of `!Send` data after issue #17667 is complete.
pub static ACCESS_KIT_ADAPTERS: RefCell<AccessKitAdapters> = const { RefCell::new(AccessKitAdapters::new()) };
}

/// Maps window entities to their `AccessKit` [`Adapter`]s.
#[derive(Default, Deref, DerefMut)]
pub struct AccessKitAdapters(pub EntityHashMap<Adapter>);

impl AccessKitAdapters {
/// Creates a new empty `AccessKitAdapters`.
pub const fn new() -> Self {
Self(EntityHashMap::new())
}
}

/// Maps window entities to their respective [`ActionRequest`]s.
#[derive(Resource, Default, Deref, DerefMut)]
pub struct WinitActionRequestHandlers(pub EntityHashMap<Arc<Mutex<WinitActionRequestHandler>>>);
Expand Down Expand Up @@ -144,14 +158,16 @@ pub(crate) fn prepare_accessibility_for_window(
}

fn window_closed(
mut adapters: NonSendMut<AccessKitAdapters>,
mut handlers: ResMut<WinitActionRequestHandlers>,
mut events: EventReader<WindowClosed>,
_non_send_marker: NonSendMarker,
) {
for WindowClosed { window, .. } in events.read() {
adapters.remove(window);
handlers.remove(window);
}
ACCESS_KIT_ADAPTERS.with_borrow_mut(|adapters| {
for WindowClosed { window, .. } in events.read() {
adapters.remove(window);
handlers.remove(window);
}
});
}

fn poll_receivers(
Expand All @@ -174,7 +190,6 @@ fn should_update_accessibility_nodes(
}

fn update_accessibility_nodes(
mut adapters: NonSendMut<AccessKitAdapters>,
focus: Option<Res<InputFocus>>,
primary_window: Query<(Entity, &Window), With<PrimaryWindow>>,
nodes: Query<(
Expand All @@ -184,35 +199,38 @@ fn update_accessibility_nodes(
Option<&ChildOf>,
)>,
node_entities: Query<Entity, With<AccessibilityNode>>,
_non_send_marker: NonSendMarker,
) {
let Ok((primary_window_id, primary_window)) = primary_window.single() else {
return;
};
let Some(adapter) = adapters.get_mut(&primary_window_id) else {
return;
};
let Some(focus) = focus else {
return;
};
if focus.is_changed() || !nodes.is_empty() {
// Don't panic if the focused entity does not currently exist
// It's probably waiting to be spawned
if let Some(focused_entity) = focus.0 {
if !node_entities.contains(focused_entity) {
return;
ACCESS_KIT_ADAPTERS.with_borrow_mut(|adapters| {
let Ok((primary_window_id, primary_window)) = primary_window.single() else {
return;
};
let Some(adapter) = adapters.get_mut(&primary_window_id) else {
return;
};
let Some(focus) = focus else {
return;
};
if focus.is_changed() || !nodes.is_empty() {
// Don't panic if the focused entity does not currently exist
// It's probably waiting to be spawned
if let Some(focused_entity) = focus.0 {
if !node_entities.contains(focused_entity) {
return;
}
}
}

adapter.update_if_active(|| {
update_adapter(
nodes,
node_entities,
primary_window,
primary_window_id,
focus,
)
});
}
adapter.update_if_active(|| {
update_adapter(
nodes,
node_entities,
primary_window,
primary_window_id,
focus,
)
});
}
});
}

fn update_adapter(
Expand Down Expand Up @@ -290,8 +308,7 @@ pub struct AccessKitPlugin;

impl Plugin for AccessKitPlugin {
fn build(&self, app: &mut App) {
app.init_non_send_resource::<AccessKitAdapters>()
.init_resource::<WinitActionRequestHandlers>()
app.init_resource::<WinitActionRequestHandlers>()
.add_event::<ActionRequestWrapper>()
.add_systems(
PostUpdate,
Expand Down
12 changes: 7 additions & 5 deletions crates/bevy_winit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use bevy_derive::Deref;
use bevy_reflect::prelude::ReflectDefault;
use bevy_reflect::Reflect;
use bevy_window::{RawHandleWrapperHolder, WindowEvent};
use core::cell::RefCell;
use core::marker::PhantomData;
use winit::{event_loop::EventLoop, window::WindowId};

Expand All @@ -37,7 +38,7 @@ pub use winit_config::*;
pub use winit_windows::*;

use crate::{
accessibility::{AccessKitAdapters, AccessKitPlugin, WinitActionRequestHandlers},
accessibility::{AccessKitPlugin, WinitActionRequestHandlers},
state::winit_runner,
winit_monitors::WinitMonitors,
};
Expand All @@ -53,6 +54,10 @@ mod winit_config;
mod winit_monitors;
mod winit_windows;

thread_local! {
static WINIT_WINDOWS: RefCell<WinitWindows> = const { RefCell::new(WinitWindows::new()) };
}

/// A [`Plugin`] that uses `winit` to create and manage windows, and receive window and input
/// events.
///
Expand Down Expand Up @@ -124,8 +129,7 @@ impl<T: Event> Plugin for WinitPlugin<T> {
.build()
.expect("Failed to build event loop");

app.init_non_send_resource::<WinitWindows>()
.init_resource::<WinitMonitors>()
app.init_resource::<WinitMonitors>()
.init_resource::<WinitSettings>()
.insert_resource(DisplayHandleWrapper(event_loop.owned_display_handle()))
.add_event::<RawWinitWindowEvent>()
Expand Down Expand Up @@ -210,8 +214,6 @@ pub type CreateWindowParams<'w, 's, F = ()> = (
F,
>,
EventWriter<'w, WindowCreated>,
NonSendMut<'w, WinitWindows>,
NonSendMut<'w, AccessKitAdapters>,
ResMut<'w, WinitActionRequestHandlers>,
Res<'w, AccessibilityRequested>,
Res<'w, WinitMonitors>,
Expand Down
Loading