diff --git a/core/executor/src/host_interface.rs b/core/executor/src/host_interface.rs
index 7d87d93333fd25c734ec0861112670c10dd27ebe..4d1515d76999b7fafcf4b899ebe90698999b89dd 100644
--- a/core/executor/src/host_interface.rs
+++ b/core/executor/src/host_interface.rs
@@ -18,10 +18,8 @@
//!
//! These are the host functions callable from within the Substrate runtime.
-use crate::error::{Error, Result};
-
use codec::Encode;
-use std::{convert::TryFrom, str, panic};
+use std::{convert::TryFrom, str};
use primitives::{
blake2_128, blake2_256, twox_64, twox_128, twox_256, ed25519, sr25519, Blake2Hasher, Pair,
crypto::KeyTypeId, offchain,
@@ -211,10 +209,7 @@ impl_wasm_host_interface! {
.map_err(|_| "Invalid attempt to determine key in ext_set_storage")?;
let value = context.read_memory(value_data, value_len)
.map_err(|_| "Invalid attempt to determine value in ext_set_storage")?;
- with_external_storage(move ||
- Ok(runtime_io::set_storage(&key, &value))
- )?;
- Ok(())
+ Ok(runtime_io::set_storage(&key, &value))
}
ext_set_child_storage(
@@ -232,10 +227,7 @@ impl_wasm_host_interface! {
let value = context.read_memory(value_data, value_len)
.map_err(|_| "Invalid attempt to determine value in ext_set_child_storage")?;
- with_external_storage(move ||
- Ok(runtime_io::set_child_storage(&storage_key, &key, &value))
- )?;
- Ok(())
+ Ok(runtime_io::set_child_storage(&storage_key, &key, &value))
}
ext_clear_child_storage(
@@ -249,27 +241,19 @@ impl_wasm_host_interface! {
let key = context.read_memory(key_data, key_len)
.map_err(|_| "Invalid attempt to determine key in ext_clear_child_storage")?;
- with_external_storage(move ||
- Ok(runtime_io::clear_child_storage(&storage_key, &key))
- )?;
- Ok(())
+ Ok(runtime_io::clear_child_storage(&storage_key, &key))
}
ext_clear_storage(key_data: Pointer<u8>, key_len: WordSize) {
let key = context.read_memory(key_data, key_len)
.map_err(|_| "Invalid attempt to determine key in ext_clear_storage")?;
- with_external_storage(move ||
- Ok(runtime_io::clear_storage(&key))
- )?;
- Ok(())
+ Ok(runtime_io::clear_storage(&key))
}
ext_exists_storage(key_data: Pointer<u8>, key_len: WordSize) -> u32 {
let key = context.read_memory(key_data, key_len)
.map_err(|_| "Invalid attempt to determine key in ext_exists_storage")?;
- with_external_storage(move ||
- Ok(if runtime_io::exists_storage(&key) { 1 } else { 0 })
- )
+ Ok(if runtime_io::exists_storage(&key) { 1 } else { 0 })
}
ext_exists_child_storage(
@@ -283,18 +267,13 @@ impl_wasm_host_interface! {
let key = context.read_memory(key_data, key_len)
.map_err(|_| "Invalid attempt to determine key in ext_exists_child_storage")?;
- with_external_storage(move ||
- Ok(if runtime_io::exists_child_storage(&storage_key, &key) { 1 } else { 0 })
- )
+ Ok(if runtime_io::exists_child_storage(&storage_key, &key) { 1 } else { 0 })
}
ext_clear_prefix(prefix_data: Pointer<u8>, prefix_len: WordSize) {
let prefix = context.read_memory(prefix_data, prefix_len)
.map_err(|_| "Invalid attempt to determine prefix in ext_clear_prefix")?;
- with_external_storage(move ||
- Ok(runtime_io::clear_prefix(&prefix))
- )?;
- Ok(())
+ Ok(runtime_io::clear_prefix(&prefix))
}
ext_clear_child_prefix(
@@ -307,21 +286,13 @@ impl_wasm_host_interface! {
.map_err(|_| "Invalid attempt to determine storage_key in ext_clear_child_prefix")?;
let prefix = context.read_memory(prefix_data, prefix_len)
.map_err(|_| "Invalid attempt to determine prefix in ext_clear_child_prefix")?;
- with_external_storage(move ||
- Ok(runtime_io::clear_child_prefix(&storage_key, &prefix))
- )?;
-
- Ok(())
+ Ok(runtime_io::clear_child_prefix(&storage_key, &prefix))
}
ext_kill_child_storage(storage_key_data: Pointer<u8>, storage_key_len: WordSize) {
let storage_key = context.read_memory(storage_key_data, storage_key_len)
.map_err(|_| "Invalid attempt to determine storage_key in ext_kill_child_storage")?;
- with_external_storage(move ||
- Ok(runtime_io::kill_child_storage(&storage_key))
- )?;
-
- Ok(())
+ Ok(runtime_io::kill_child_storage(&storage_key))
}
ext_get_allocated_storage(
@@ -331,11 +302,8 @@ impl_wasm_host_interface! {
) -> Pointer<u8> {
let key = context.read_memory(key_data, key_len)
.map_err(|_| "Invalid attempt to determine key in ext_get_allocated_storage")?;
- let maybe_value = with_external_storage(move ||
- Ok(runtime_io::storage(&key))
- )?;
- if let Some(value) = maybe_value {
+ if let Some(value) = runtime_io::storage(&key) {
let offset = context.allocate_memory(value.len() as u32)?;
context.write_memory(offset, &value)
.map_err(|_| "Invalid attempt to set memory in ext_get_allocated_storage")?;
@@ -361,11 +329,7 @@ impl_wasm_host_interface! {
let key = context.read_memory(key_data, key_len)
.map_err(|_| "Invalid attempt to determine key in ext_get_allocated_child_storage")?;
- let maybe_value = with_external_storage(move ||
- Ok(runtime_io::child_storage(&storage_key, &key))
- )?;
-
- if let Some(value) = maybe_value {
+ if let Some(value) = runtime_io::child_storage(&storage_key, &key) {
let offset = context.allocate_memory(value.len() as u32)?;
context.write_memory(offset, &value)
.map_err(|_| "Invalid attempt to set memory in ext_get_allocated_child_storage")?;
@@ -388,11 +352,8 @@ impl_wasm_host_interface! {
) -> WordSize {
let key = context.read_memory(key_data, key_len)
.map_err(|_| "Invalid attempt to get key in ext_get_storage_into")?;
- let maybe_value = with_external_storage(move ||
- Ok(runtime_io::storage(&key))
- )?;
- if let Some(value) = maybe_value {
+ if let Some(value) = runtime_io::storage(&key) {
let data = &value[value.len().min(value_offset as usize)..];
let written = std::cmp::min(value_len as usize, data.len());
context.write_memory(value_data, &data[..written])
@@ -417,11 +378,7 @@ impl_wasm_host_interface! {
let key = context.read_memory(key_data, key_len)
.map_err(|_| "Invalid attempt to get key in ext_get_child_storage_into")?;
- let maybe_value = with_external_storage(move ||
- Ok(runtime_io::child_storage(&storage_key, &key))
- )?;
-
- if let Some(value) = maybe_value {
+ if let Some(value) = runtime_io::child_storage(&storage_key, &key) {
let data = &value[value.len().min(value_offset as usize)..];
let written = std::cmp::min(value_len as usize, data.len());
context.write_memory(value_data, &data[..written])
@@ -433,12 +390,8 @@ impl_wasm_host_interface! {
}
ext_storage_root(result: Pointer<u8>) {
- let r = with_external_storage(move ||
- Ok(runtime_io::storage_root())
- )?;
- context.write_memory(result, r.as_ref())
- .map_err(|_| "Invalid attempt to set memory in ext_storage_root")?;
- Ok(())
+ context.write_memory(result, runtime_io::storage_root().as_ref())
+ .map_err(|_| "Invalid attempt to set memory in ext_storage_root".into())
}
ext_child_storage_root(
@@ -448,9 +401,7 @@ impl_wasm_host_interface! {
) -> Pointer<u8> {
let storage_key = context.read_memory(storage_key_data, storage_key_len)
.map_err(|_| "Invalid attempt to determine storage_key in ext_child_storage_root")?;
- let value = with_external_storage(move ||
- Ok(runtime_io::child_storage_root(&storage_key))
- )?;
+ let value = runtime_io::child_storage_root(&storage_key);
let offset = context.allocate_memory(value.len() as u32)?;
context.write_memory(offset, &value)
@@ -468,11 +419,8 @@ impl_wasm_host_interface! {
let mut parent_hash = [0u8; 32];
context.read_memory_into(parent_hash_data, &mut parent_hash[..])
.map_err(|_| "Invalid attempt to get parent_hash in ext_storage_changes_root")?;
- let r = with_external_storage(move ||
- Ok(runtime_io::storage_changes_root(parent_hash))
- )?;
- if let Some(r) = r {
+ if let Some(r) = runtime_io::storage_changes_root(parent_hash) {
context.write_memory(result, &r[..])
.map_err(|_| "Invalid attempt to set memory in ext_storage_changes_root")?;
Ok(1)
@@ -1146,25 +1094,6 @@ impl ReadPrimitive<u32> for &mut dyn FunctionContext {
}
}
-/// Execute closure that access external storage.
-///
-/// All panics that happen within closure are captured and transformed into
-/// runtime error. This requires special panic handler mode to be enabled
-/// during the call (see `panic_handler::AbortGuard::never_abort`).
-/// If this mode isn't enabled, then all panics within externalities are
-/// leading to process abort.
-fn with_external_storage<T, F>(f: F) -> std::result::Result<T, String>
- where
- F: panic::UnwindSafe + FnOnce() -> Result<T>
-{
- // it is safe beause basic methods of StorageExternalities are guaranteed to touch only
- // its internal state + we should discard it on error
- panic::catch_unwind(move || f())
- .map_err(|_| Error::Runtime)
- .and_then(|result| result)
- .map_err(|err| format!("{}", err))
-}
-
fn deadline_to_timestamp(deadline: u64) -> Option<offchain::Timestamp> {
if deadline == 0 {
None
diff --git a/core/executor/src/native_executor.rs b/core/executor/src/native_executor.rs
index 7323703ed96568af9a8e79f8aadd55a9338b794a..082f0ba2adc330eb23033539f3461f13ca7380e6 100644
--- a/core/executor/src/native_executor.rs
+++ b/core/executor/src/native_executor.rs
@@ -14,7 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Substrate. If not, see <http://www.gnu.org/licenses/>.
-use std::{result, cell::RefCell, panic::UnwindSafe};
+use std::{result, cell::RefCell, panic::{UnwindSafe, AssertUnwindSafe}};
use crate::error::{Error, Result};
use crate::wasm_runtime::{RuntimesCache, WasmExecutionMethod, WasmRuntime};
use crate::RuntimeInfo;
@@ -30,7 +30,7 @@ thread_local! {
/// Default num of pages for the heap
const DEFAULT_HEAP_PAGES: u64 = 1024;
-fn safe_call<F, U>(f: F) -> Result<U>
+pub(crate) fn safe_call<F, U>(f: F) -> Result<U>
where F: UnwindSafe + FnOnce() -> U
{
// Substrate uses custom panic hook that terminates process on panic. Disable termination for the native call.
@@ -65,7 +65,7 @@ pub trait NativeExecutionDispatch: Send + Sync {
#[derive(Debug)]
pub struct NativeExecutor<D> {
/// Dummy field to avoid the compiler complaining about us not using `D`.
- _dummy: ::std::marker::PhantomData<D>,
+ _dummy: std::marker::PhantomData<D>,
/// Method used to execute fallback Wasm code.
fallback_method: WasmExecutionMethod,
/// Native runtime version info.
@@ -92,15 +92,45 @@ impl<D: NativeExecutionDispatch> NativeExecutor<D> {
}
}
+ /// Execute the given closure `f` with the latest runtime (based on the `CODE` key in `ext`).
+ ///
+ /// The closure `f` is expected to return `Err(_)` when there happened a `panic!` in native code
+ /// while executing the runtime in Wasm. If a `panic!` occurred, the runtime is invalidated to
+ /// prevent any poisoned state. Native runtime execution does not need to report back
+ /// any `panic!`.
+ ///
+ /// # Safety
+ ///
+ /// `runtime` and `ext` are given as `AssertUnwindSafe` to the closure. As described above, the
+ /// runtime is invalidated on any `panic!` to prevent a poisoned state. `ext` is already
+ /// implicitly handled as unwind safe, as we store it in a global variable while executing the
+ /// native runtime.
fn with_runtime<E, R>(
&self,
ext: &mut E,
- f: impl for <'a> FnOnce(&'a mut dyn WasmRuntime, &'a mut E) -> Result<R>,
+ f: impl for<'a> FnOnce(
+ AssertUnwindSafe<&'a mut (dyn WasmRuntime + 'static)>,
+ AssertUnwindSafe<&'a mut E>,
+ ) -> Result<Result<R>>,
) -> Result<R> where E: Externalities {
RUNTIMES_CACHE.with(|cache| {
let mut cache = cache.borrow_mut();
- let runtime = cache.fetch_runtime(ext, self.fallback_method, self.default_heap_pages)?;
- f(runtime, ext)
+ let (runtime, code_hash) = cache.fetch_runtime(
+ ext,
+ self.fallback_method,
+ self.default_heap_pages,
+ )?;
+
+ let runtime = AssertUnwindSafe(runtime);
+ let ext = AssertUnwindSafe(ext);
+
+ match f(runtime, ext) {
+ Ok(res) => res,
+ Err(e) => {
+ cache.invalidate_runtime(self.fallback_method, code_hash);
+ Err(e)
+ }
+ }
})
}
}
@@ -125,7 +155,7 @@ impl<D: NativeExecutionDispatch> RuntimeInfo for NativeExecutor<D> {
&self,
ext: &mut E,
) -> Option<RuntimeVersion> {
- match self.with_runtime(ext, |runtime, _ext| Ok(runtime.version())) {
+ match self.with_runtime(ext, |runtime, _ext| Ok(Ok(runtime.version()))) {
Ok(version) => version,
Err(e) => {
warn!(target: "executor", "Failed to fetch runtime: {:?}", e);
@@ -141,8 +171,8 @@ impl<D: NativeExecutionDispatch> CodeExecutor for NativeExecutor<D> {
fn call
<
E: Externalities,
- R:Decode + Encode + PartialEq,
- NC: FnOnce() -> result::Result<R, String> + UnwindSafe
+ R: Decode + Encode + PartialEq,
+ NC: FnOnce() -> result::Result<R, String> + UnwindSafe,
>(
&self,
ext: &mut E,
@@ -152,7 +182,7 @@ impl<D: NativeExecutionDispatch> CodeExecutor for NativeExecutor<D> {
native_call: Option<NC>,
) -> (Result<NativeOrEncoded<R>>, bool){
let mut used_native = false;
- let result = self.with_runtime(ext, |runtime, ext| {
+ let result = self.with_runtime(ext, |mut runtime, mut ext| {
let onchain_version = runtime.version();
match (
use_native,
@@ -170,9 +200,16 @@ impl<D: NativeExecutionDispatch> CodeExecutor for NativeExecutor<D> {
.as_ref()
.map_or_else(||"<None>".into(), |v| format!("{}", v))
);
- runtime.call(ext, method, data).map(NativeOrEncoded::Encoded)
+
+ safe_call(
+ move || runtime.call(&mut **ext, method, data).map(NativeOrEncoded::Encoded)
+ )
}
- (false, _, _) => runtime.call(ext, method, data).map(NativeOrEncoded::Encoded),
+ (false, _, _) => {
+ safe_call(
+ move || runtime.call(&mut **ext, method, data).map(NativeOrEncoded::Encoded)
+ )
+ },
(true, true, Some(call)) => {
trace!(
target: "executor",
@@ -184,11 +221,13 @@ impl<D: NativeExecutionDispatch> CodeExecutor for NativeExecutor<D> {
);
used_native = true;
- with_native_environment(ext, move || (call)())
+ let res = with_native_environment(&mut **ext, move || (call)())
.and_then(|r| r
.map(NativeOrEncoded::Native)
.map_err(|s| Error::ApiError(s.to_string()))
- )
+ );
+
+ Ok(res)
}
_ => {
trace!(
@@ -199,7 +238,7 @@ impl<D: NativeExecutionDispatch> CodeExecutor for NativeExecutor<D> {
);
used_native = true;
- D::dispatch(ext, method, data).map(NativeOrEncoded::Encoded)
+ Ok(D::dispatch(&mut **ext, method, data).map(NativeOrEncoded::Encoded))
}
}
});
diff --git a/core/executor/src/wasm_runtime.rs b/core/executor/src/wasm_runtime.rs
index 8d2291fe048932cedf247cf5c05c02a5f8d5a8fd..29a6c51e39d3b1dce1258c3b5e9b442bb504bd12 100644
--- a/core/executor/src/wasm_runtime.rs
+++ b/core/executor/src/wasm_runtime.rs
@@ -23,7 +23,7 @@ use crate::error::{Error, WasmError};
use crate::wasmi_execution;
use log::{trace, warn};
use codec::Decode;
-use primitives::{storage::well_known_keys, traits::Externalities};
+use primitives::{storage::well_known_keys, traits::Externalities, H256};
use runtime_version::RuntimeVersion;
use std::{collections::hash_map::{Entry, HashMap}};
@@ -99,9 +99,8 @@ impl RuntimesCache {
///
/// # Return value
///
- /// If no error occurred a tuple `(wasmi::ModuleRef, Option<RuntimeVersion>)` is
- /// returned. `RuntimeVersion` is contained if the call to `Core_version` returned
- /// a version.
+ /// If no error occurred a tuple `(&mut WasmRuntime, H256)` is
+ /// returned. `H256` is the hash of the runtime code.
///
/// In case of failure one of two errors can be returned:
///
@@ -114,7 +113,7 @@ impl RuntimesCache {
ext: &mut E,
wasm_method: WasmExecutionMethod,
default_heap_pages: u64,
- ) -> Result<&mut (dyn WasmRuntime + 'static), Error> {
+ ) -> Result<(&mut (dyn WasmRuntime + 'static), H256), Error> {
let code_hash = ext
.original_storage_hash(well_known_keys::CODE)
.ok_or(Error::InvalidCode("`CODE` not found in storage.".into()))?;
@@ -131,7 +130,7 @@ impl RuntimesCache {
if !cached_runtime.update_heap_pages(heap_pages) {
trace!(
target: "runtimes_cache",
- "heap_pages were changed. Reinstantiating the instance"
+ "heap_pages were changed. Reinstantiating the instance",
);
*result = create_wasm_runtime(ext, wasm_method, heap_pages);
if let Err(ref err) = result {
@@ -152,9 +151,23 @@ impl RuntimesCache {
};
result.as_mut()
- .map(|runtime| runtime.as_mut())
+ .map(|runtime| (runtime.as_mut(), code_hash))
.map_err(|ref e| Error::InvalidCode(format!("{:?}", e)))
}
+
+ /// Invalidate the runtime for the given `wasm_method` and `code_hash`.
+ ///
+ /// Invalidation of a runtime is useful when there was a `panic!` in native while executing it.
+ /// The `panic!` maybe have brought the runtime into a poisoned state and so, it is better to
+ /// invalidate this runtime instance.
+ pub fn invalidate_runtime(
+ &mut self,
+ wasm_method: WasmExecutionMethod,
+ code_hash: H256,
+ ) {
+ // Just remove the instance, it will be re-created the next time it is requested.
+ self.instances.remove(&(wasm_method, code_hash.into()));
+ }
}
/// Create a wasm runtime with the given `code`.
diff --git a/core/executor/src/wasmi_execution.rs b/core/executor/src/wasmi_execution.rs
index 2b832b490641de116ce03dc1adec41025f1d9c60..a83729b4b6c8d37cfeb8cc2a6709a9e7dab40a9b 100644
--- a/core/executor/src/wasmi_execution.rs
+++ b/core/executor/src/wasmi_execution.rs
@@ -16,7 +16,7 @@
//! Implementation of a Wasm runtime using the Wasmi interpreter.
-use std::{str, mem};
+use std::{str, mem, panic::AssertUnwindSafe};
use wasmi::{
Module, ModuleInstance, MemoryInstance, MemoryRef, TableRef, ImportsBuilder, ModuleRef,
memory_units::Pages, RuntimeValue::{I32, I64, self},
@@ -625,9 +625,14 @@ pub fn create_instance<E: Externalities>(ext: &mut E, code: &[u8], heap_pages: u
",
);
- let version = call_in_wasm_module(ext, &instance, "Core_version", &[])
- .ok()
- .and_then(|v| RuntimeVersion::decode(&mut v.as_slice()).ok());
+ let mut ext = AssertUnwindSafe(ext);
+ let call_instance = AssertUnwindSafe(&instance);
+ let version = crate::native_executor::safe_call(
+ move || call_in_wasm_module(&mut **ext, *call_instance, "Core_version", &[])
+ .ok()
+ .and_then(|v| RuntimeVersion::decode(&mut v.as_slice()).ok())
+ ).map_err(WasmError::Instantiation)?;
+
Ok(WasmiRuntime {
instance,
version,
diff --git a/core/wasm-interface/src/lib.rs b/core/wasm-interface/src/lib.rs
index b3cbde556eea0dbfe6c985c089706d9469a9d1e0..1e672606389007a690de92665522a96dd36b259b 100644
--- a/core/wasm-interface/src/lib.rs
+++ b/core/wasm-interface/src/lib.rs
@@ -180,7 +180,7 @@ pub trait Function {
fn execute(
&self,
context: &mut dyn FunctionContext,
- args: &mut dyn Iterator<Item=Value>,
+ args: &mut dyn Iterator<Item = Value>,
) -> Result<Option<Value>>;
}