Skip to content

Commit

Permalink
Allow access to non-send resource through World::resource_scope (be…
Browse files Browse the repository at this point in the history
…vyengine#6113)

# Objective

Relaxes the trait bound for `World::resource_scope` to allow non-send resources. Fixes bevyengine#6037.

## Solution

No big changes in code had to be made. Added a check so that the non-send resources won't be accessed from a different thread.

---

## Changelog
 - `World::resource_scope` accepts non-send resources now
 - `World::resource_scope` verifies non-send access if the resource is non-send
 - Two new tests are added, one for valid use of `World::resource_scope` with a non-send resource, and one for invalid use (calling it from a different thread, resulting in panic)

Co-authored-by: Dawid Piotrowski <41804418+Pietrek14@users.noreply.github.com>
  • Loading branch information
2 people authored and james7132 committed Oct 28, 2022
1 parent d6d07a6 commit aee2ccd
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 1 deletion.
36 changes: 36 additions & 0 deletions crates/bevy_ecs/src/lib.rs
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
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. */
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

0 comments on commit aee2ccd

Please sign in to comment.