Skip to content

[Merged by Bors] - Allow access to non-send resource through World::resource_scope #6113

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

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 36 additions & 0 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ mod tests {
use bevy_tasks::{ComputeTaskPool, TaskPool};
use std::{
any::TypeId,
marker::PhantomData,
sync::{
atomic::{AtomicUsize, Ordering},
Arc, Mutex,
Expand All @@ -78,6 +79,9 @@ mod tests {
#[derive(Component, Debug, PartialEq, Eq, Clone, Copy)]
struct C;

#[derive(Default)]
struct NonSendA(usize, PhantomData<*mut ()>);

#[derive(Component, Clone, Debug)]
struct DropCk(Arc<AtomicUsize>);
impl DropCk {
Expand Down Expand Up @@ -1262,6 +1266,38 @@ mod tests {
assert_eq!(world.resource::<A>().0, 1);
}

#[test]
fn non_send_resource_scope() {
let mut world = World::default();
world.insert_non_send_resource(NonSendA::default());
world.resource_scope(|world: &mut World, mut value: Mut<NonSendA>| {
value.0 += 1;
assert!(!world.contains_resource::<NonSendA>());
});
assert_eq!(world.non_send_resource::<NonSendA>().0, 1);
}

#[test]
#[should_panic(
expected = "attempted to access NonSend resource bevy_ecs::tests::NonSendA off of the main thread"
)]
fn non_send_resource_scope_from_different_thread() {
let mut world = World::default();
world.insert_non_send_resource(NonSendA::default());

let thread = std::thread::spawn(move || {
// Accessing the non-send resource on a different thread
// Should result in a panic
world.resource_scope(|_: &mut World, mut value: Mut<NonSendA>| {
value.0 += 1;
});
});

if let Err(err) = thread.join() {
std::panic::resume_unwind(err);
}
}

#[test]
fn insert_overwrite_drop() {
let (dropck1, dropped1) = DropCk::new_pair();
Expand Down
15 changes: 14 additions & 1 deletion crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1149,14 +1149,27 @@ impl World {
/// });
/// assert_eq!(world.get_resource::<A>().unwrap().0, 2);
/// ```
pub fn resource_scope<R: Resource, U>(&mut self, f: impl FnOnce(&mut World, Mut<R>) -> U) -> U {
pub fn resource_scope<
R: 'static, /* The resource doesn't need to be Send nor Sync. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really we should remove the Send/Sync supertraits on resources, because the fact that you can add anything as a non send resource is a bit messy.

But this is fine for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should open another issue about it, since it's out of the scope of this PR.

U,
>(
&mut self,
f: impl FnOnce(&mut World, Mut<R>) -> U,
) -> U {
let last_change_tick = self.last_change_tick();
let change_tick = self.change_tick();

let component_id = self
.components
.get_resource_id(TypeId::of::<R>())
.unwrap_or_else(|| panic!("resource does not exist: {}", std::any::type_name::<R>()));

// If the resource isn't send and sync, validate that we are on the main thread, so that we can access it.
let component_info = self.components().get_info(component_id).unwrap();
if !component_info.is_send_and_sync() {
self.validate_non_send_access::<R>();
}

let (ptr, mut ticks) = {
let resource_archetype = self.archetypes.resource_mut();
let unique_components = resource_archetype.unique_components_mut();
Expand Down