From e00ae1ea73b8d619e18e34289a4861146cf1b21a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 22 Oct 2023 00:21:27 +0200 Subject: [PATCH 1/2] Bump aes-gcm from 0.10.2 to 0.10.3 (#1893) Bumps [aes-gcm](https://github.com/RustCrypto/AEADs) from 0.10.2 to 0.10.3.
Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=aes-gcm&package-manager=cargo&previous-version=0.10.2&new-version=0.10.3)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/paritytech/polkadot-sdk/network/alerts).
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0267a0c1e55f9..1ea3eb38d0cd4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -121,9 +121,9 @@ dependencies = [ [[package]] name = "aes-gcm" -version = "0.10.2" +version = "0.10.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "209b47e8954a928e1d72e86eca7000ebb6655fe1436d33eefc2201cad027e237" +checksum = "831010a0f742e1209b3bcea8fab6a8e149051ba6099432c8cb2cc117dec3ead1" dependencies = [ "aead 0.5.2", "aes 0.8.3", @@ -17420,7 +17420,7 @@ dependencies = [ name = "sp-statement-store" version = "4.0.0-dev" dependencies = [ - "aes-gcm 0.10.2", + "aes-gcm 0.10.3", "curve25519-dalek 4.0.0", "ed25519-dalek", "hkdf", @@ -19962,7 +19962,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4a00f4242f2db33307347bd5be53263c52a0331c96c14292118c9a6bb48d267" dependencies = [ "aes 0.6.0", - "aes-gcm 0.10.2", + "aes-gcm 0.10.3", "async-trait", "bincode", "block-modes", From e2b21d00c7856961ab3b692b2e2798747af1e819 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sun, 22 Oct 2023 10:44:34 +0200 Subject: [PATCH 2/2] sc-executor: Increase maximum instance count (#1856) Changes the maximum instances count for `wasmtime` to `64`. It also allows to only pass in maximum `32` for `--max-runtime-instances` as `256` was way too big. With `64` instances in total and `32` that can be configured in maximum, there should be enough space to accommodate for extra instances that are may required to be allocated adhoc. --------- Co-authored-by: Koute --- Cargo.lock | 1 + .../client/cli/src/params/runtime_params.rs | 6 +- substrate/client/executor/wasmtime/Cargo.toml | 1 + .../executor/wasmtime/src/instance_wrapper.rs | 17 ++++- .../client/executor/wasmtime/src/runtime.rs | 64 ++++++++++++++++++- 5 files changed, 80 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1ea3eb38d0cd4..ae88e4c2c992b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15172,6 +15172,7 @@ dependencies = [ "libc", "log", "parity-scale-codec", + "parking_lot 0.12.1", "paste", "rustix 0.36.15", "sc-allocator", diff --git a/substrate/client/cli/src/params/runtime_params.rs b/substrate/client/cli/src/params/runtime_params.rs index 79035fc7d4c5d..07009a96ee6e6 100644 --- a/substrate/client/cli/src/params/runtime_params.rs +++ b/substrate/client/cli/src/params/runtime_params.rs @@ -22,7 +22,7 @@ use std::str::FromStr; /// Parameters used to config runtime. #[derive(Debug, Clone, Args)] pub struct RuntimeParams { - /// The size of the instances cache for each runtime. The values higher than 256 are illegal. + /// The size of the instances cache for each runtime. The values higher than 32 are illegal. #[arg(long, default_value_t = 8, value_parser = parse_max_runtime_instances)] pub max_runtime_instances: usize, @@ -35,8 +35,8 @@ fn parse_max_runtime_instances(s: &str) -> Result { let max_runtime_instances = usize::from_str(s) .map_err(|_err| format!("Illegal `--max-runtime-instances` value: {s}"))?; - if max_runtime_instances > 256 { - Err(format!("Illegal `--max-runtime-instances` value: {max_runtime_instances} is more than the allowed maximum of `256` ")) + if max_runtime_instances > 32 { + Err(format!("Illegal `--max-runtime-instances` value: {max_runtime_instances} is more than the allowed maximum of `32` ")) } else { Ok(max_runtime_instances) } diff --git a/substrate/client/executor/wasmtime/Cargo.toml b/substrate/client/executor/wasmtime/Cargo.toml index fee1afc96666b..261d52c0ede32 100644 --- a/substrate/client/executor/wasmtime/Cargo.toml +++ b/substrate/client/executor/wasmtime/Cargo.toml @@ -16,6 +16,7 @@ targets = ["x86_64-unknown-linux-gnu"] log = "0.4.17" cfg-if = "1.0" libc = "0.2.121" +parking_lot = "0.12.1" # When bumping wasmtime do not forget to also bump rustix # to exactly the same version as used by wasmtime! diff --git a/substrate/client/executor/wasmtime/src/instance_wrapper.rs b/substrate/client/executor/wasmtime/src/instance_wrapper.rs index acc799061c27c..8852532adbcaa 100644 --- a/substrate/client/executor/wasmtime/src/instance_wrapper.rs +++ b/substrate/client/executor/wasmtime/src/instance_wrapper.rs @@ -19,7 +19,9 @@ //! Defines data and logic needed for interaction with an WebAssembly instance of a substrate //! runtime module. -use crate::runtime::{Store, StoreData}; +use std::sync::Arc; + +use crate::runtime::{InstanceCounter, ReleaseInstanceHandle, Store, StoreData}; use sc_executor_common::{ error::{Backtrace, Error, MessageWithBacktrace, Result, WasmError}, wasm_runtime::InvokeMethod, @@ -154,10 +156,19 @@ impl sc_allocator::Memory for MemoryWrapper<'_, C> { pub struct InstanceWrapper { instance: Instance, store: Store, + // NOTE: We want to decrement the instance counter *after* the store has been dropped + // to avoid a potential race condition, so this field must always be kept + // as the last field in the struct! + _release_instance_handle: ReleaseInstanceHandle, } impl InstanceWrapper { - pub(crate) fn new(engine: &Engine, instance_pre: &InstancePre) -> Result { + pub(crate) fn new( + engine: &Engine, + instance_pre: &InstancePre, + instance_counter: Arc, + ) -> Result { + let _release_instance_handle = instance_counter.acquire_instance(); let mut store = Store::new(engine, Default::default()); let instance = instance_pre.instantiate(&mut store).map_err(|error| { WasmError::Other(format!( @@ -172,7 +183,7 @@ impl InstanceWrapper { store.data_mut().memory = Some(memory); store.data_mut().table = table; - Ok(InstanceWrapper { instance, store }) + Ok(InstanceWrapper { instance, store, _release_instance_handle }) } /// Resolves a substrate entrypoint by the given name. diff --git a/substrate/client/executor/wasmtime/src/runtime.rs b/substrate/client/executor/wasmtime/src/runtime.rs index ae78137959be1..ac88663f4e791 100644 --- a/substrate/client/executor/wasmtime/src/runtime.rs +++ b/substrate/client/executor/wasmtime/src/runtime.rs @@ -24,6 +24,7 @@ use crate::{ util::{self, replace_strategy_if_broken}, }; +use parking_lot::Mutex; use sc_allocator::{AllocationStats, FreeingBumpHeapAllocator}; use sc_executor_common::{ error::{Error, Result, WasmError}, @@ -42,6 +43,8 @@ use std::{ }; use wasmtime::{AsContext, Engine, Memory, Table}; +const MAX_INSTANCE_COUNT: u32 = 64; + #[derive(Default)] pub(crate) struct StoreData { /// This will only be set when we call into the runtime. @@ -73,11 +76,59 @@ enum Strategy { struct InstanceCreator { engine: Engine, instance_pre: Arc>, + instance_counter: Arc, } impl InstanceCreator { fn instantiate(&mut self) -> Result { - InstanceWrapper::new(&self.engine, &self.instance_pre) + InstanceWrapper::new(&self.engine, &self.instance_pre, self.instance_counter.clone()) + } +} + +/// A handle for releasing an instance acquired by [`InstanceCounter::acquire_instance`]. +pub(crate) struct ReleaseInstanceHandle { + counter: Arc, +} + +impl Drop for ReleaseInstanceHandle { + fn drop(&mut self) { + { + let mut counter = self.counter.counter.lock(); + *counter = counter.saturating_sub(1); + } + + self.counter.wait_for_instance.notify_one(); + } +} + +/// Keeps track on the number of parallel instances. +/// +/// The runtime cache keeps track on the number of parallel instances. The maximum number in the +/// cache is less than what we have configured as [`MAX_INSTANCE_COUNT`] for wasmtime. However, the +/// cache will create on demand instances if required. This instance counter will ensure that we are +/// blocking when we are trying to create too many instances. +#[derive(Default)] +pub(crate) struct InstanceCounter { + counter: Mutex, + wait_for_instance: parking_lot::Condvar, +} + +impl InstanceCounter { + /// Acquire an instance. + /// + /// Blocks if there is no free instance available. + /// + /// The returned [`ReleaseInstanceHandle`] should be dropped when the instance isn't used + /// anymore. + pub fn acquire_instance(self: Arc) -> ReleaseInstanceHandle { + let mut counter = self.counter.lock(); + + while *counter >= MAX_INSTANCE_COUNT { + self.wait_for_instance.wait(&mut counter); + } + *counter += 1; + + ReleaseInstanceHandle { counter: self.clone() } } } @@ -87,6 +138,7 @@ pub struct WasmtimeRuntime { engine: Engine, instance_pre: Arc>, instantiation_strategy: InternalInstantiationStrategy, + instance_counter: Arc, } impl WasmModule for WasmtimeRuntime { @@ -95,6 +147,7 @@ impl WasmModule for WasmtimeRuntime { InternalInstantiationStrategy::Builtin => Strategy::RecreateInstance(InstanceCreator { engine: self.engine.clone(), instance_pre: self.instance_pre.clone(), + instance_counter: self.instance_counter.clone(), }), }; @@ -277,7 +330,7 @@ fn common_config(semantics: &Semantics) -> std::result::Result