From 97c146fbdcf356854f27b92490fcd1f1c1819757 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Wed, 1 Apr 2020 17:33:04 +0200 Subject: [PATCH] Fix problems with tests run in parallel (https://github.com/enso-org/ide/pull/332) Global spawner was made thread local. Original commit: https://github.com/enso-org/ide/commit/8931df9b44f1efd805d3a026b51235ae0e01b539 --- gui/src/rust/ide/src/executor/global.rs | 46 +++++++++++++------------ gui/src/rust/ide/src/model/module.rs | 29 +++++++++------- 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/gui/src/rust/ide/src/executor/global.rs b/gui/src/rust/ide/src/executor/global.rs index 4135f8d75b..4041aaa90e 100644 --- a/gui/src/rust/ide/src/executor/global.rs +++ b/gui/src/rust/ide/src/executor/global.rs @@ -26,11 +26,16 @@ use crate::prelude::*; use futures::task::LocalSpawnExt; use futures::task::LocalSpawn; -/// Global spawner handle. -/// -/// It should be set up once, as part of the IDE initialization routine and -/// remain accessible indefinitely. -static mut SPAWNER: Option> = None; +thread_local! { + /// Global spawner handle. + /// + /// It should be set up once, as part of the IDE initialization routine and + /// remain accessible indefinitely. + /// + /// This is made thread local for tests which may be run in parallel; Each test should set + /// executor independently. + static SPAWNER: RefCell>> = default(); +} /// Sets the global spawner. It will remain accessible until it is set again to /// something else. @@ -40,29 +45,26 @@ static mut SPAWNER: Option> = None; #[allow(unsafe_code)] pub fn set_spawner(spawner_to_set:impl LocalSpawn + 'static) { // Note [Global Executor Safety] - unsafe { - SPAWNER = Some(Box::new(spawner_to_set)); - } + SPAWNER.with(|s| *s.borrow_mut() = Some(Box::new(spawner_to_set))); } -// Note [Global Executor Safety] -// ============================= -// This is safe, because the global mutable state is only accessed through the -// two functions provided in this module, the code will be used only in a web -// single-threaded environment (so no race conditions are possible), and the -// functions do not leak reference to the global spawner. - /// Spawns a task using the global spawner. /// Panics, if called when there is no global spawner set or if it fails to /// spawn task (e.g. because the connected executor was prematurely dropped). #[allow(unsafe_code)] pub fn spawn(f:impl Future + 'static) { - // Note [Global Executor Safety] - let spawner = unsafe { + SPAWNER.with(|spawner| { let error_msg = "No global executor has been provided."; - SPAWNER.as_mut().expect(error_msg) - }; - - let error_msg = "Failed to spawn the task. Global executor might have been dropped."; - spawner.spawn_local(f).expect(error_msg); + // Note [Global Executor Safety] + let mut borrowed = spawner.borrow_mut(); + let unwrapped = borrowed.as_mut().expect(error_msg); + let error_msg = "Failed to spawn the task. Global executor might have been dropped."; + unwrapped.spawn_local(f).expect(error_msg); + }); } + +// Note [Global Executor Safety] +// ============================= +// This borrowing is safe, because the global mutable state is only accessed through the +// two functions provided in this module, and the functions do not leak reference to the global +// spawner. diff --git a/gui/src/rust/ide/src/model/module.rs b/gui/src/rust/ide/src/model/module.rs index 6635ac5303..c90da6dfb4 100644 --- a/gui/src/rust/ide/src/model/module.rs +++ b/gui/src/rust/ide/src/model/module.rs @@ -219,7 +219,7 @@ mod test { #[test] fn notifying() { - let mut test = TestWithLocalPoolExecutor::set_up(); + let mut test = TestWithLocalPoolExecutor::set_up(); test.run_task(async { let module = Module::default(); let mut text_subscription = module.subscribe_text_notifications(); @@ -259,21 +259,24 @@ mod test { #[test] fn handling_metadata() { - let module = Module::default(); + let mut test = TestWithLocalPoolExecutor::set_up(); + test.run_task(async { + let module = Module::default(); - let id = Uuid::new_v4(); - let initial_md = module.node_metadata(id.clone()); - assert!(initial_md.is_err()); + let id = Uuid::new_v4(); + let initial_md = module.node_metadata(id.clone()); + assert!(initial_md.is_err()); - let md_to_set = NodeMetadata {position:Some(Position::new(1.0, 2.0))}; - module.set_node_metadata(id.clone(),md_to_set.clone()); - assert_eq!(md_to_set.position, module.node_metadata(id.clone()).unwrap().position); + let md_to_set = NodeMetadata {position:Some(Position::new(1.0, 2.0))}; + module.set_node_metadata(id.clone(),md_to_set.clone()); + assert_eq!(md_to_set.position, module.node_metadata(id.clone()).unwrap().position); - let new_pos = Position::new(4.0, 5.0); - module.with_node_metadata(id.clone(), |md| { - assert_eq!(md_to_set.position, md.position); - md.position = Some(new_pos); + let new_pos = Position::new(4.0, 5.0); + module.with_node_metadata(id.clone(), |md| { + assert_eq!(md_to_set.position, md.position); + md.position = Some(new_pos); + }); + assert_eq!(Some(new_pos), module.node_metadata(id).unwrap().position); }); - assert_eq!(Some(new_pos), module.node_metadata(id).unwrap().position); } }