From 769bff96fb367348e040e98e73ee18cfb8b1d663 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Sat, 1 Jan 2022 23:22:16 +0000 Subject: [PATCH 1/4] Prototype world being `!Send` --- crates/bevy_core_pipeline/src/lib.rs | 5 ++- crates/bevy_ecs/src/component.rs | 7 ++++ crates/bevy_ecs/src/lib.rs | 12 ------ crates/bevy_ecs/src/world/mod.rs | 39 ++++++++++++++++++- crates/bevy_gltf/src/loader.rs | 8 +++- crates/bevy_render/src/lib.rs | 10 ++--- .../src/render_resource/pipeline_cache.rs | 4 +- crates/bevy_render/src/view/window.rs | 2 +- crates/bevy_scene/src/dynamic_scene.rs | 2 +- crates/bevy_scene/src/scene.rs | 10 +---- crates/bevy_scene/src/scene_spawner.rs | 8 ++-- crates/bevy_sprite/src/render/mod.rs | 4 +- crates/bevy_text/src/text2d.rs | 4 +- crates/bevy_ui/src/render/mod.rs | 4 +- 14 files changed, 76 insertions(+), 43 deletions(-) diff --git a/crates/bevy_core_pipeline/src/lib.rs b/crates/bevy_core_pipeline/src/lib.rs index 88b06f93cbb75..f1e55e3cebc6c 100644 --- a/crates/bevy_core_pipeline/src/lib.rs +++ b/crates/bevy_core_pipeline/src/lib.rs @@ -327,7 +327,10 @@ impl CachedPipelinePhaseItem for Transparent3d { } } -pub fn extract_clear_color(clear_color: Res, mut render_world: ResMut) { +pub fn extract_clear_color( + clear_color: Res, + mut render_world: NonSendMut, +) { // If the clear color has changed if clear_color.is_changed() { // Update the clear color resource in the render world diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 69b3254dea8cb..2dc656edb478d 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -347,6 +347,13 @@ impl Components { ComponentId(*index) } + + pub(crate) fn non_send_components(&'_ self) -> impl Iterator + '_ { + self.components + .iter() + .filter(|x| !x.is_send_and_sync()) + .map(|x| x.id()) + } } #[derive(Clone, Debug)] diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 30d70ca17efab..607933883c962 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1193,18 +1193,6 @@ mod tests { assert_eq!(*world.get_non_send_resource_mut::().unwrap(), 456); } - #[test] - #[should_panic] - fn non_send_resource_panic() { - let mut world = World::default(); - world.insert_non_send(0i32); - std::thread::spawn(move || { - let _ = world.get_non_send_resource_mut::(); - }) - .join() - .unwrap(); - } - #[test] fn trackers_query() { let mut world = World::default(); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 1087181adfe4c..35234213259f8 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1142,6 +1142,17 @@ impl World { self.archetypes.clear_entities(); self.entities.clear(); } + + pub fn turtle(self) -> Turtle { + let non_send = self.components().non_send_components().collect::>(); + for id in non_send { + assert!( + self.get_populated_resource_column(id).is_some(), + "Tried to create a Turtle from a World containing a !Send resource" + ); + } + Turtle { world: self } + } } impl fmt::Debug for World { @@ -1159,9 +1170,35 @@ impl fmt::Debug for World { } } -unsafe impl Send for World {} unsafe impl Sync for World {} +/// A world which does not contain any [`!Send`] resources, and therefore +/// can be safely sent between threads +/// +/// [`!Send`]: Send +#[derive(Debug)] +pub struct Turtle { + world: World, +} + +unsafe impl Send for Turtle {} + +impl Turtle { + pub fn world(self) -> World { + self.world + } + + pub fn world_ref(&self) -> &World { + &self.world + } +} + +impl Into for Turtle { + fn into(self) -> World { + self.world() + } +} + /// Creates `Self` using data from the given [World] pub trait FromWorld { /// Creates `Self` using data from the given [World] diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index db59b3a8f25ba..626913dcdf98f 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -302,8 +302,12 @@ async fn load_gltf<'a, 'b>( if let Some(Err(err)) = err { return Err(err); } - let scene_handle = load_context - .set_labeled_asset(&scene_label(&scene), LoadedAsset::new(Scene::new(world))); + let scene_handle = load_context.set_labeled_asset( + &scene_label(&scene), + LoadedAsset::new(Scene { + turtle: world.turtle(), + }), + ); if let Some(name) = scene.name() { named_scenes.insert(name.to_string(), scene_handle.clone()); diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index 788dd9365b48f..9f2a8e2f3d7a8 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -144,7 +144,7 @@ impl Plugin for RenderPlugin { app.insert_resource(device.clone()) .insert_resource(queue.clone()) .insert_resource(options.clone()) - .init_resource::() + .init_non_send_resource::() .register_type::() .register_type::(); let render_pipeline_cache = RenderPipelineCache::new(device.clone()); @@ -303,16 +303,16 @@ fn extract(app_world: &mut World, render_app: &mut App) { .unwrap(); // temporarily add the render world to the app world as a resource - let scratch_world = app_world.remove_resource::().unwrap(); + let scratch_world = app_world.remove_non_send::().unwrap(); let render_world = std::mem::replace(&mut render_app.world, scratch_world.0); - app_world.insert_resource(RenderWorld(render_world)); + app_world.insert_non_send(RenderWorld(render_world)); extract.run(app_world); // add the render world back to the render app - let render_world = app_world.remove_resource::().unwrap(); + let render_world = app_world.remove_non_send::().unwrap(); let scratch_world = std::mem::replace(&mut render_app.world, render_world.0); - app_world.insert_resource(ScratchRenderWorld(scratch_world)); + app_world.insert_non_send(ScratchRenderWorld(scratch_world)); extract.apply_buffers(&mut render_app.world); } diff --git a/crates/bevy_render/src/render_resource/pipeline_cache.rs b/crates/bevy_render/src/render_resource/pipeline_cache.rs index e5fcede3cbec9..05969498fd622 100644 --- a/crates/bevy_render/src/render_resource/pipeline_cache.rs +++ b/crates/bevy_render/src/render_resource/pipeline_cache.rs @@ -9,7 +9,7 @@ use crate::{ }; use bevy_app::EventReader; use bevy_asset::{AssetEvent, Assets, Handle}; -use bevy_ecs::system::{Res, ResMut}; +use bevy_ecs::system::{NonSendMut, Res, ResMut}; use bevy_utils::{tracing::error, HashMap, HashSet}; use std::{collections::hash_map::Entry, hash::Hash, ops::Deref, sync::Arc}; use thiserror::Error; @@ -389,7 +389,7 @@ impl RenderPipelineCache { } pub(crate) fn extract_shaders( - mut world: ResMut, + mut world: NonSendMut, shaders: Res>, mut events: EventReader>, ) { diff --git a/crates/bevy_render/src/view/window.rs b/crates/bevy_render/src/view/window.rs index 94bf8a21c0fdb..83284b9d211f5 100644 --- a/crates/bevy_render/src/view/window.rs +++ b/crates/bevy_render/src/view/window.rs @@ -67,7 +67,7 @@ impl DerefMut for ExtractedWindows { } } -fn extract_windows(mut render_world: ResMut, windows: Res) { +fn extract_windows(mut render_world: NonSendMut, windows: Res) { let mut extracted_windows = render_world.get_resource_mut::().unwrap(); for window in windows.iter() { let (new_width, new_height) = ( diff --git a/crates/bevy_scene/src/dynamic_scene.rs b/crates/bevy_scene/src/dynamic_scene.rs index ab15cbd61dbdd..2dc215d422861 100644 --- a/crates/bevy_scene/src/dynamic_scene.rs +++ b/crates/bevy_scene/src/dynamic_scene.rs @@ -27,7 +27,7 @@ pub struct DynamicEntity { impl DynamicScene { /// Create a new dynamic scene from a given scene. pub fn from_scene(scene: &Scene, type_registry: &TypeRegistryArc) -> Self { - Self::from_world(&scene.world, type_registry) + Self::from_world(&scene.turtle.world_ref(), type_registry) } /// Create a new dynamic scene from a given world. diff --git a/crates/bevy_scene/src/scene.rs b/crates/bevy_scene/src/scene.rs index 0a8bd50b82492..8082a4a9a9e35 100644 --- a/crates/bevy_scene/src/scene.rs +++ b/crates/bevy_scene/src/scene.rs @@ -1,14 +1,8 @@ -use bevy_ecs::world::World; +use bevy_ecs::world::Turtle; use bevy_reflect::TypeUuid; #[derive(Debug, TypeUuid)] #[uuid = "c156503c-edd9-4ec7-8d33-dab392df03cd"] pub struct Scene { - pub world: World, -} - -impl Scene { - pub fn new(world: World) -> Self { - Self { world } - } + pub turtle: Turtle, } diff --git a/crates/bevy_scene/src/scene_spawner.rs b/crates/bevy_scene/src/scene_spawner.rs index 7015195757d79..f65af56dd3367 100644 --- a/crates/bevy_scene/src/scene_spawner.rs +++ b/crates/bevy_scene/src/scene_spawner.rs @@ -154,15 +154,15 @@ impl SceneSpawner { handle: scene_handle.clone(), })?; - for archetype in scene.world.archetypes().iter() { + let scene_world = scene.turtle.world_ref(); + for archetype in scene_world.archetypes().iter() { for scene_entity in archetype.entities() { let entity = *instance_info .entity_map .entry(*scene_entity) .or_insert_with(|| world.spawn().id()); for component_id in archetype.components() { - let component_info = scene - .world + let component_info = scene_world .components() .get_info(component_id) .expect("component_ids in archetypes should have ComponentInfo"); @@ -180,7 +180,7 @@ impl SceneSpawner { }) })?; reflect_component.copy_component( - &scene.world, + &scene_world, world, *scene_entity, entity, diff --git a/crates/bevy_sprite/src/render/mod.rs b/crates/bevy_sprite/src/render/mod.rs index f4d0f035a98bb..a8b828c6c57c9 100644 --- a/crates/bevy_sprite/src/render/mod.rs +++ b/crates/bevy_sprite/src/render/mod.rs @@ -204,7 +204,7 @@ pub struct SpriteAssetEvents { } pub fn extract_sprite_events( - mut render_world: ResMut, + mut render_world: NonSendMut, mut image_events: EventReader>, ) { let mut events = render_world @@ -230,7 +230,7 @@ pub fn extract_sprite_events( } pub fn extract_sprites( - mut render_world: ResMut, + mut render_world: NonSendMut, texture_atlases: Res>, sprite_query: Query<(&Visibility, &Sprite, &GlobalTransform, &Handle)>, atlas_query: Query<( diff --git a/crates/bevy_text/src/text2d.rs b/crates/bevy_text/src/text2d.rs index bee204e755fa3..44e976b492f08 100644 --- a/crates/bevy_text/src/text2d.rs +++ b/crates/bevy_text/src/text2d.rs @@ -3,7 +3,7 @@ use bevy_ecs::{ bundle::Bundle, entity::Entity, query::{Changed, QueryState, With}, - system::{Local, Query, QuerySet, Res, ResMut}, + system::{Local, NonSendMut, Query, QuerySet, Res, ResMut}, }; use bevy_math::{Size, Vec3}; use bevy_render::{texture::Image, view::Visibility, RenderWorld}; @@ -42,7 +42,7 @@ impl Default for Text2dBundle { } pub fn extract_text2d_sprite( - mut render_world: ResMut, + mut render_world: NonSendMut, texture_atlases: Res>, text_pipeline: Res, windows: Res, diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index e632b4eee18cb..b6e064117fdc1 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -136,7 +136,7 @@ pub struct ExtractedUiNodes { } pub fn extract_uinodes( - mut render_world: ResMut, + mut render_world: NonSendMut, images: Res>, uinode_query: Query<( &Node, @@ -173,7 +173,7 @@ pub fn extract_uinodes( } pub fn extract_text_uinodes( - mut render_world: ResMut, + mut render_world: NonSendMut, texture_atlases: Res>, text_pipeline: Res, windows: Res, From 22b40330b68d3843d0e225ba0b898f115ea4be4c Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Sat, 1 Jan 2022 23:29:38 +0000 Subject: [PATCH 2/4] Add docs and fix inverted condition --- crates/bevy_ecs/src/world/mod.rs | 36 ++++++++++++++++++++++---- crates/bevy_scene/src/dynamic_scene.rs | 2 +- crates/bevy_scene/src/scene_spawner.rs | 7 +---- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 35234213259f8..e3083b5958af0 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1143,14 +1143,25 @@ impl World { self.entities.clear(); } + /// Create a version of this [`World`] which can be sent to another thread + /// + /// # Panics + /// + /// If `self` contains any [`!Send`] resources, e.g. from calls to [`World::insert_non_send`] + /// + /// [`!Send`]: Send + pub fn turtle(self) -> Turtle { let non_send = self.components().non_send_components().collect::>(); for id in non_send { assert!( - self.get_populated_resource_column(id).is_some(), + self.get_populated_resource_column(id).is_none(), "Tried to create a Turtle from a World containing a !Send resource" ); } + // Safety: this world does not contain anything !Send, as confirmed by the check above + // In practise this method is used for GLTF loading, which does not add any resources to the given world + // (i.e. the above check is trivially true) Turtle { world: self } } } @@ -1173,29 +1184,44 @@ impl fmt::Debug for World { unsafe impl Sync for World {} /// A world which does not contain any [`!Send`] resources, and therefore -/// can be safely sent between threads +/// can be safely sent between threads. +/// +/// The name turtle is derived from this being something which is moving a +/// [`World`] (between threads) /// /// [`!Send`]: Send #[derive(Debug)] pub struct Turtle { + // Safety: does not have any !Send resources world: World, } +// Safety: The contained world does not contain anything which is !Send unsafe impl Send for Turtle {} impl Turtle { + /// The [`World`] this [`Turtle`] was created from. + /// + /// The returned [`World`] does not contain any [`!Send`] resources + /// + /// [`!Send`]: Send pub fn world(self) -> World { self.world } + /// A view on this [`Turtle`]'s [`World`], which contains no [`!Send`] resources + /// + /// [`!Send`]: Send + // Safety: NonSend resources cannot be added using a raw reference + // to the world, so this cannot break our invariants pub fn world_ref(&self) -> &World { &self.world } } -impl Into for Turtle { - fn into(self) -> World { - self.world() +impl From for World { + fn from(turtle: Turtle) -> Self { + turtle.world() } } diff --git a/crates/bevy_scene/src/dynamic_scene.rs b/crates/bevy_scene/src/dynamic_scene.rs index 2dc215d422861..e75a308aaca94 100644 --- a/crates/bevy_scene/src/dynamic_scene.rs +++ b/crates/bevy_scene/src/dynamic_scene.rs @@ -27,7 +27,7 @@ pub struct DynamicEntity { impl DynamicScene { /// Create a new dynamic scene from a given scene. pub fn from_scene(scene: &Scene, type_registry: &TypeRegistryArc) -> Self { - Self::from_world(&scene.turtle.world_ref(), type_registry) + Self::from_world(scene.turtle.world_ref(), type_registry) } /// Create a new dynamic scene from a given world. diff --git a/crates/bevy_scene/src/scene_spawner.rs b/crates/bevy_scene/src/scene_spawner.rs index f65af56dd3367..2f9914eef0dbd 100644 --- a/crates/bevy_scene/src/scene_spawner.rs +++ b/crates/bevy_scene/src/scene_spawner.rs @@ -179,12 +179,7 @@ impl SceneSpawner { } }) })?; - reflect_component.copy_component( - &scene_world, - world, - *scene_entity, - entity, - ); + reflect_component.copy_component(scene_world, world, *scene_entity, entity); } } } From 896dffe92529a6dfc424e25b1052b5a2e6a6c10f Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Tue, 11 Jan 2022 09:14:02 +0000 Subject: [PATCH 3/4] raw->shared --- crates/bevy_ecs/src/world/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index e3083b5958af0..99fd34bba5f4b 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1212,7 +1212,7 @@ impl Turtle { /// A view on this [`Turtle`]'s [`World`], which contains no [`!Send`] resources /// /// [`!Send`]: Send - // Safety: NonSend resources cannot be added using a raw reference + // Safety: NonSend resources cannot be added using a shared reference // to the world, so this cannot break our invariants pub fn world_ref(&self) -> &World { &self.world From 32e7743849e793dea2d06b0ac694e2c8efa22b11 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Thu, 13 Jan 2022 20:40:41 +0000 Subject: [PATCH 4/4] =?UTF-8?q?Appease=20the=20=F0=9F=97=9E?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/bevy_ecs/src/world/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 99fd34bba5f4b..d6fb1872866d0 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1196,6 +1196,8 @@ pub struct Turtle { world: World, } +// I mean I guess clippy, that's the point. There's a reason we called it `unsafe` +#[allow(clippy::non_send_fields_in_send_ty)] // Safety: The contained world does not contain anything which is !Send unsafe impl Send for Turtle {}