Remove sp_tasks::spawn API and related code + host functions (#12639)

* Remove `sp_tasks::spawn` API and related code

* Remove `RuntimeTasks::{spawn, join}` host functions

* remove unused

* Remove a few more tests that I forgot to remove

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

substrate/client/executor/Cargo.toml

@@ -31,7 +31,6 @@ sp-externalities = { version = "0.12.0", path = "../../primitives/externalities"
 sp-io = { version = "6.0.0", path = "../../primitives/io" }
 sp-panic-handler = { version = "4.0.0", path = "../../primitives/panic-handler" }
 sp-runtime-interface = { version = "6.0.0", path = "../../primitives/runtime-interface" }
-sp-tasks = { version = "4.0.0-dev", path = "../../primitives/tasks" }
 sp-trie = { version = "6.0.0", path = "../../primitives/trie" }
 sp-version = { version = "5.0.0", path = "../../primitives/version" }
 sp-wasm-interface = { version = "6.0.0", path = "../../primitives/wasm-interface" }

substrate/client/executor/runtime-test/Cargo.toml

@@ -19,7 +19,6 @@ sp-io = { version = "6.0.0", default-features = false, features = ["improved_pan
 sp-runtime = { version = "6.0.0", default-features = false, path = "../../../primitives/runtime" }
 sp-sandbox = { version = "0.10.0-dev", default-features = false, path = "../../../primitives/sandbox" }
 sp-std = { version = "4.0.0", default-features = false, path = "../../../primitives/std" }
-sp-tasks = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/tasks" }
 
 [build-dependencies]
 substrate-wasm-builder = { version = "5.0.0-dev", path = "../../../utils/wasm-builder" }
@@ -32,5 +31,4 @@ std = [
 	"sp-runtime/std",
 	"sp-sandbox/std",
 	"sp-std/std",
-	"sp-tasks/std",
 ]

substrate/client/executor/runtime-test/src/lib.rs

@@ -318,24 +318,6 @@ sp_core::wasm_export_functions! {
 		message_slice.copy_from_slice(test_message);
 	}
 
-	fn test_spawn() {
-		let data = vec![1u8, 2u8];
-		let data_new = sp_tasks::spawn(tasks::incrementer, data).join();
-
-		assert_eq!(data_new, vec![2u8, 3u8]);
-	}
-
-	fn test_nested_spawn() {
-		let data = vec![7u8, 13u8];
-		let data_new = sp_tasks::spawn(tasks::parallel_incrementer, data).join();
-
-		assert_eq!(data_new, vec![10u8, 16u8]);
-	}
-
-	fn test_panic_in_spawned() {
-		sp_tasks::spawn(tasks::panicker, vec![]).join();
-	}
-
 	fn test_return_i8() -> i8 {
 		-66
 	}
@@ -358,25 +340,6 @@ sp_core::wasm_export_functions! {
 	}
 }
 
-#[cfg(not(feature = "std"))]
-mod tasks {
-	use sp_std::prelude::*;
-
-	pub fn incrementer(data: Vec<u8>) -> Vec<u8> {
-		data.into_iter().map(|v| v + 1).collect()
-	}
-
-	pub fn panicker(_: Vec<u8>) -> Vec<u8> {
-		panic!()
-	}
-
-	pub fn parallel_incrementer(data: Vec<u8>) -> Vec<u8> {
-		let first = data.into_iter().map(|v| v + 2).collect::<Vec<_>>();
-		let second = sp_tasks::spawn(incrementer, first).join();
-		second
-	}
-}
-
 /// A macro to define a test entrypoint for each available sandbox executor.
 macro_rules! wasm_export_sandbox_test_functions {
 	(

substrate/client/executor/src/integration_tests/mod.rs

@@ -770,33 +770,6 @@ fn wasm_tracing_should_work(wasm_method: WasmExecutionMethod) {
 	assert_eq!(len, 2);
 }
 
-test_wasm_execution!(spawning_runtime_instance_should_work);
-fn spawning_runtime_instance_should_work(wasm_method: WasmExecutionMethod) {
-	let mut ext = TestExternalities::default();
-	let mut ext = ext.ext();
-
-	call_in_wasm("test_spawn", &[], wasm_method, &mut ext).unwrap();
-}
-
-test_wasm_execution!(spawning_runtime_instance_nested_should_work);
-fn spawning_runtime_instance_nested_should_work(wasm_method: WasmExecutionMethod) {
-	let mut ext = TestExternalities::default();
-	let mut ext = ext.ext();
-
-	call_in_wasm("test_nested_spawn", &[], wasm_method, &mut ext).unwrap();
-}
-
-test_wasm_execution!(panic_in_spawned_instance_panics_on_joining_its_result);
-fn panic_in_spawned_instance_panics_on_joining_its_result(wasm_method: WasmExecutionMethod) {
-	let mut ext = TestExternalities::default();
-	let mut ext = ext.ext();
-
-	let error_result =
-		call_in_wasm("test_panic_in_spawned", &[], wasm_method, &mut ext).unwrap_err();
-
-	assert!(error_result.to_string().contains("Spawned task"));
-}
-
 test_wasm_execution!(memory_is_cleared_between_invocations);
 fn memory_is_cleared_between_invocations(wasm_method: WasmExecutionMethod) {
 	// This is based on the code generated by compiling a runtime *without*

substrate/client/executor/src/native_executor.rs

@@ -23,24 +23,18 @@ use crate::{
 };
 
 use std::{
-	collections::HashMap,
 	marker::PhantomData,
 	panic::{AssertUnwindSafe, UnwindSafe},
 	path::PathBuf,
-	sync::{
-		atomic::{AtomicU64, Ordering},
-		mpsc, Arc,
-	},
+	sync::Arc,
 };
 
 use codec::Encode;
 use sc_executor_common::{
 	runtime_blob::RuntimeBlob,
-	wasm_runtime::{AllocationStats, InvokeMethod, WasmInstance, WasmModule},
+	wasm_runtime::{AllocationStats, WasmInstance, WasmModule},
 };
-use sp_core::traits::{CodeExecutor, Externalities, RuntimeCode, RuntimeSpawn, RuntimeSpawnExt};
-use sp_externalities::ExternalitiesExt as _;
-use sp_tasks::new_async_externalities;
+use sp_core::traits::{CodeExecutor, Externalities, RuntimeCode};
 use sp_version::{GetNativeVersion, NativeVersion, RuntimeVersion};
 use sp_wasm_interface::{ExtendedHostFunctions, HostFunctions};
 
@@ -277,11 +271,9 @@ where
 
 		let mut instance = AssertUnwindSafe(instance);
 		let mut ext = AssertUnwindSafe(ext);
-		let module = AssertUnwindSafe(module);
 		let mut allocation_stats_out = AssertUnwindSafe(allocation_stats_out);
 
 		with_externalities_safe(&mut **ext, move || {
-			preregister_builtin_ext(module.clone());
 			let (result, allocation_stats) =
 				instance.call_with_allocation_stats(export_name.into(), call_data);
 			**allocation_stats_out = allocation_stats;
@@ -349,16 +341,10 @@ where
 			"Executing function",
 		);
 
-		let result = self.with_instance(
-			runtime_code,
-			ext,
-			|module, mut instance, _onchain_version, mut ext| {
-				with_externalities_safe(&mut **ext, move || {
-					preregister_builtin_ext(module.clone());
-					instance.call_export(method, data)
-				})
-			},
-		);
+		let result =
+			self.with_instance(runtime_code, ext, |_, mut instance, _onchain_version, mut ext| {
+				with_externalities_safe(&mut **ext, move || instance.call_export(method, data))
+			});
 		(result, false)
 	}
 }
@@ -451,138 +437,6 @@ impl<D: NativeExecutionDispatch> GetNativeVersion for NativeElseWasmExecutor<D>
 	}
 }
 
-/// Helper inner struct to implement `RuntimeSpawn` extension.
-pub struct RuntimeInstanceSpawn {
-	module: Arc<dyn WasmModule>,
-	tasks: parking_lot::Mutex<HashMap<u64, mpsc::Receiver<Vec<u8>>>>,
-	counter: AtomicU64,
-	scheduler: Box<dyn sp_core::traits::SpawnNamed>,
-}
-
-impl RuntimeSpawn for RuntimeInstanceSpawn {
-	fn spawn_call(&self, dispatcher_ref: u32, func: u32, data: Vec<u8>) -> u64 {
-		let new_handle = self.counter.fetch_add(1, Ordering::Relaxed);
-
-		let (sender, receiver) = mpsc::channel();
-		self.tasks.lock().insert(new_handle, receiver);
-
-		let module = self.module.clone();
-		let scheduler = self.scheduler.clone();
-		self.scheduler.spawn(
-			"executor-extra-runtime-instance",
-			None,
-			Box::pin(async move {
-				let module = AssertUnwindSafe(module);
-
-				let async_ext = match new_async_externalities(scheduler.clone()) {
-					Ok(val) => val,
-					Err(e) => {
-						tracing::error!(
-							target: "executor",
-							error = %e,
-							"Failed to setup externalities for async context.",
-						);
-
-						// This will drop sender and receiver end will panic
-						return
-					},
-				};
-
-				let mut async_ext = match async_ext.with_runtime_spawn(Box::new(
-					RuntimeInstanceSpawn::new(module.clone(), scheduler),
-				)) {
-					Ok(val) => val,
-					Err(e) => {
-						tracing::error!(
-							target: "executor",
-							error = %e,
-							"Failed to setup runtime extension for async externalities",
-						);
-
-						// This will drop sender and receiver end will panic
-						return
-					},
-				};
-
-				let result = with_externalities_safe(&mut async_ext, move || {
-					// FIXME: Should be refactored to shared "instance factory".
-					// Instantiating wasm here every time is suboptimal at the moment, shared
-					// pool of instances should be used.
-					//
-					// https://github.com/paritytech/substrate/issues/7354
-					let mut instance = match module.new_instance() {
-						Ok(instance) => instance,
-						Err(error) => {
-							panic!("failed to create new instance from module: {}", error)
-						},
-					};
-
-					match instance
-						.call(InvokeMethod::TableWithWrapper { dispatcher_ref, func }, &data[..])
-					{
-						Ok(result) => result,
-						Err(error) => panic!("failed to invoke instance: {}", error),
-					}
-				});
-
-				match result {
-					Ok(output) => {
-						let _ = sender.send(output);
-					},
-					Err(error) => {
-						// If execution is panicked, the `join` in the original runtime code will
-						// panic as well, since the sender is dropped without sending anything.
-						tracing::error!(error = %error, "Call error in spawned task");
-					},
-				}
-			}),
-		);
-
-		new_handle
-	}
-
-	fn join(&self, handle: u64) -> Vec<u8> {
-		let receiver = self.tasks.lock().remove(&handle).expect("No task for the handle");
-		receiver.recv().expect("Spawned task panicked for the handle")
-	}
-}
-
-impl RuntimeInstanceSpawn {
-	pub fn new(
-		module: Arc<dyn WasmModule>,
-		scheduler: Box<dyn sp_core::traits::SpawnNamed>,
-	) -> Self {
-		Self { module, scheduler, counter: 0.into(), tasks: HashMap::new().into() }
-	}
-
-	fn with_externalities_and_module(
-		module: Arc<dyn WasmModule>,
-		mut ext: &mut dyn Externalities,
-	) -> Option<Self> {
-		ext.extension::<sp_core::traits::TaskExecutorExt>()
-			.map(move |task_ext| Self::new(module, task_ext.clone()))
-	}
-}
-
-/// Pre-registers the built-in extensions to the currently effective externalities.
-///
-/// Meant to be called each time before calling into the runtime.
-fn preregister_builtin_ext(module: Arc<dyn WasmModule>) {
-	sp_externalities::with_externalities(move |mut ext| {
-		if let Some(runtime_spawn) =
-			RuntimeInstanceSpawn::with_externalities_and_module(module, ext)
-		{
-			if let Err(e) = ext.register_extension(RuntimeSpawnExt(Box::new(runtime_spawn))) {
-				tracing::trace!(
-					target: "executor",
-					error = ?e,
-					"Failed to register `RuntimeSpawnExt` instance on externalities",
-				)
-			}
-		}
-	});
-}
-
 impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeElseWasmExecutor<D> {
 	type Error = Error;
 
@@ -604,7 +458,7 @@ impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeElseWasmExecut
 		let result = self.wasm.with_instance(
 			runtime_code,
 			ext,
-			|module, mut instance, onchain_version, mut ext| {
+			|_, mut instance, onchain_version, mut ext| {
 				let onchain_version =
 					onchain_version.ok_or_else(|| Error::ApiError("Unknown version".into()))?;
 
@@ -632,10 +486,7 @@ impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeElseWasmExecut
 						);
 					}
 
-					with_externalities_safe(&mut **ext, move || {
-						preregister_builtin_ext(module.clone());
-						instance.call_export(method, data)
-					})
+					with_externalities_safe(&mut **ext, move || instance.call_export(method, data))
 				}
 			},
 		);

substrate/client/executor/wasmtime/src/runtime.rs

@@ -395,9 +395,6 @@ fn common_config(semantics: &Semantics) -> std::result::Result<wasmtime::Config,
 
 				// This determines how many instances of the module can be
 				// instantiated in parallel from the same `Module`.
-				//
-				// This includes nested instances spawned with `sp_tasks::spawn`
-				// from *within* the runtime.
 				count: 32,
 			},
 		});