From 4da5acab32c8f526fd3133d9c7cd1e482aeb1933 Mon Sep 17 00:00:00 2001 From: james7132 Date: Tue, 1 Nov 2022 15:52:34 -0700 Subject: [PATCH 1/9] Basic panic handling --- crates/bevy_tasks/src/task_pool.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/bevy_tasks/src/task_pool.rs b/crates/bevy_tasks/src/task_pool.rs index d15a9e6ff2a75..f64a1ca88ac53 100644 --- a/crates/bevy_tasks/src/task_pool.rs +++ b/crates/bevy_tasks/src/task_pool.rs @@ -118,14 +118,22 @@ impl TaskPool { thread_builder .spawn(move || { TaskPool::LOCAL_EXECUTOR.with(|local_executor| { - let tick_forever = async move { - loop { - local_executor.tick().await; + loop { + let res = std::panic::catch_unwind(|| { + let tick_forever = async move { + loop { + local_executor.tick().await; + } + }; + // Use unwrap_err because we expect a Closed error + future::block_on(ex.run(tick_forever.or(shutdown_rx.recv()))) + }); + match res { + // Recieved an error from shutdown's recv. We should terminate. + Ok(Err(_)) => break, + _ => {}, } - }; - let shutdown_future = ex.run(tick_forever.or(shutdown_rx.recv())); - // Use unwrap_err because we expect a Closed error - future::block_on(shutdown_future).unwrap_err(); + } }); }) .expect("Failed to spawn thread.") From c1b79871031a63dd5767a9460833ae3485d359f9 Mon Sep 17 00:00:00 2001 From: james7132 Date: Tue, 1 Nov 2022 16:28:08 -0700 Subject: [PATCH 2/9] Add branch for handling panics --- crates/bevy_tasks/src/task_pool.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/bevy_tasks/src/task_pool.rs b/crates/bevy_tasks/src/task_pool.rs index f64a1ca88ac53..c9950364cfc6d 100644 --- a/crates/bevy_tasks/src/task_pool.rs +++ b/crates/bevy_tasks/src/task_pool.rs @@ -131,6 +131,9 @@ impl TaskPool { match res { // Recieved an error from shutdown's recv. We should terminate. Ok(Err(_)) => break, + // The executor panicked for whatever reason. + // TODO: Properly handle this. + Err(_) => {}, _ => {}, } } From da36773a506b791776bd6eaa36c910efce79a135 Mon Sep 17 00:00:00 2001 From: james7132 Date: Tue, 1 Nov 2022 17:01:24 -0700 Subject: [PATCH 3/9] Formatting --- crates/bevy_tasks/src/task_pool.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_tasks/src/task_pool.rs b/crates/bevy_tasks/src/task_pool.rs index c9950364cfc6d..b1c03c2574b2c 100644 --- a/crates/bevy_tasks/src/task_pool.rs +++ b/crates/bevy_tasks/src/task_pool.rs @@ -133,8 +133,8 @@ impl TaskPool { Ok(Err(_)) => break, // The executor panicked for whatever reason. // TODO: Properly handle this. - Err(_) => {}, - _ => {}, + Err(_) => {} + _ => {} } } }); From c6fd0271a044b4c63c2038003646d6e35cf9c301 Mon Sep 17 00:00:00 2001 From: james7132 Date: Tue, 1 Nov 2022 22:20:26 -0700 Subject: [PATCH 4/9] Rework the error handling of catch_unwind --- crates/bevy_tasks/src/task_pool.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/bevy_tasks/src/task_pool.rs b/crates/bevy_tasks/src/task_pool.rs index b1c03c2574b2c..e5bda2b9aa49a 100644 --- a/crates/bevy_tasks/src/task_pool.rs +++ b/crates/bevy_tasks/src/task_pool.rs @@ -125,16 +125,12 @@ impl TaskPool { local_executor.tick().await; } }; - // Use unwrap_err because we expect a Closed error future::block_on(ex.run(tick_forever.or(shutdown_rx.recv()))) }); - match res { - // Recieved an error from shutdown's recv. We should terminate. - Ok(Err(_)) => break, - // The executor panicked for whatever reason. - // TODO: Properly handle this. - Err(_) => {} - _ => {} + if let Ok(value) = res { + // Use unwrap_err because we expect a Closed error + value.unwrap_err(); + break; } } }); From 70a0cc94bfc73659011822d60f3fabfc2bb09b4a Mon Sep 17 00:00:00 2001 From: james7132 Date: Tue, 1 Nov 2022 23:08:04 -0700 Subject: [PATCH 5/9] Replace single_threaded -> parallel --- crates/bevy_ecs/src/system/system_piping.rs | 4 ++-- crates/bevy_time/src/time.rs | 2 +- crates/bevy_transform/src/systems.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_piping.rs b/crates/bevy_ecs/src/system/system_piping.rs index 4f4a192ff535d..c9be4096c75b1 100644 --- a/crates/bevy_ecs/src/system/system_piping.rs +++ b/crates/bevy_ecs/src/system/system_piping.rs @@ -202,7 +202,7 @@ pub mod adapter { /// // Building a new schedule/app... /// # use bevy_ecs::schedule::SystemStage; /// # let mut sched = Schedule::default(); sched - /// # .add_stage(CoreStage::Update, SystemStage::single_threaded()) + /// # .add_stage(CoreStage::Update, SystemStage::parallel()) /// .add_system_to_stage( /// CoreStage::Update, /// // Panic if the load system returns an error. @@ -246,7 +246,7 @@ pub mod adapter { /// // Building a new schedule/app... /// # use bevy_ecs::schedule::SystemStage; /// # let mut sched = Schedule::default(); sched - /// # .add_stage(CoreStage::Update, SystemStage::single_threaded()) + /// # .add_stage(CoreStage::Update, SystemStage::parallel()) /// .add_system_to_stage( /// CoreStage::Update, /// // If the system fails, just move on and try again next frame. diff --git a/crates/bevy_time/src/time.rs b/crates/bevy_time/src/time.rs index 89ef1c11a31fd..111ba6f74574d 100644 --- a/crates/bevy_time/src/time.rs +++ b/crates/bevy_time/src/time.rs @@ -120,7 +120,7 @@ impl Time { /// world.insert_resource(time); /// world.insert_resource(Health { health_value: 0.2 }); /// - /// let mut update_stage = SystemStage::single_threaded(); + /// let mut update_stage = SystemStage::parallel(); /// update_stage.add_system(health_system); /// /// // Simulate that 30 ms have passed diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 1d1c5cff45ced..d025af8dca65c 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -320,7 +320,7 @@ mod test { // Adding the system in a single threaded stage. As the system will panic, this will // only bring down the current test thread. - app.add_stage("single", SystemStage::single_threaded()) + app.add_stage("single", SystemStage::parallel()) .add_system_to_stage("single", transform_propagate_system); fn setup_world(world: &mut World) -> (Entity, Entity) { From 35d3b2d465b8f715b97c468079f0f37291cb03f2 Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 2 Nov 2022 12:38:21 -0700 Subject: [PATCH 6/9] Remove outdated comment. --- crates/bevy_transform/src/systems.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index d025af8dca65c..0a342c9d41cf1 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -318,8 +318,6 @@ mod test { let mut temp = World::new(); let mut app = App::new(); - // Adding the system in a single threaded stage. As the system will panic, this will - // only bring down the current test thread. app.add_stage("single", SystemStage::parallel()) .add_system_to_stage("single", transform_propagate_system); From da5b9ef36c0e3c638d20ab91fa1cf0cee240432b Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 2 Nov 2022 13:47:55 -0700 Subject: [PATCH 7/9] Remove unnecessary stage from test. --- crates/bevy_transform/src/systems.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 0a342c9d41cf1..5f38683650e6c 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -318,8 +318,7 @@ mod test { let mut temp = World::new(); let mut app = App::new(); - app.add_stage("single", SystemStage::parallel()) - .add_system_to_stage("single", transform_propagate_system); + app.add_system_to_stage("single", transform_propagate_system); fn setup_world(world: &mut World) -> (Entity, Entity) { let mut grandchild = Entity::from_raw(0); From 71a59ccd7b27b67b72a54fe8b8c1f3a0a670ff39 Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 2 Nov 2022 13:49:05 -0700 Subject: [PATCH 8/9] Derp --- crates/bevy_transform/src/systems.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 5f38683650e6c..6b802f585bee9 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -318,7 +318,7 @@ mod test { let mut temp = World::new(); let mut app = App::new(); - app.add_system_to_stage("single", transform_propagate_system); + app.add_system(transform_propagate_system); fn setup_world(world: &mut World) -> (Entity, Entity) { let mut grandchild = Entity::from_raw(0); From 4b7bbfef9e2fd6fa5f7ef5d1fa74b679848226f4 Mon Sep 17 00:00:00 2001 From: james7132 Date: Wed, 2 Nov 2022 16:30:05 -0700 Subject: [PATCH 9/9] Revert bevy_transform changes --- crates/bevy_transform/src/systems.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 6b802f585bee9..f96b56d676447 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -318,7 +318,10 @@ mod test { let mut temp = World::new(); let mut app = App::new(); - app.add_system(transform_propagate_system); + // FIXME: Parallel executors seem to have some odd interaction with the other + // tests in this crate. Using single_threaded until a root cause can be found. + app.add_stage("single", SystemStage::single_threaded()) + .add_system_to_stage("single", transform_propagate_system); fn setup_world(world: &mut World) -> (Entity, Entity) { let mut grandchild = Entity::from_raw(0);