Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

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

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 0 additions & 30 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ members = [
"frame/election-provider-support/solution-type/fuzzer",
"frame/examples/basic",
"frame/examples/offchain-worker",
"frame/examples/parallel",
"frame/executive",
"frame/gilt",
"frame/grandpa",
Expand Down Expand Up @@ -204,7 +203,6 @@ members = [
"primitives/state-machine",
"primitives/std",
"primitives/storage",
"primitives/tasks",
"primitives/test-primitives",
"primitives/timestamp",
"primitives/tracing",
Expand Down
1 change: 0 additions & 1 deletion client/executor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
2 changes: 0 additions & 2 deletions client/executor/runtime-test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand All @@ -32,5 +31,4 @@ std = [
"sp-runtime/std",
"sp-sandbox/std",
"sp-std/std",
"sp-tasks/std",
]
37 changes: 0 additions & 37 deletions client/executor/runtime-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
(
Expand Down
167 changes: 9 additions & 158 deletions client/executor/src/native_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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;

Expand All @@ -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()))?;

Expand Down Expand Up @@ -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))
}
},
);
Expand Down
3 changes: 0 additions & 3 deletions client/executor/wasmtime/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
});
Expand Down
Loading