From 943a555645ddc58493de81e06bc729b82454e084 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Tue, 16 Nov 2021 16:56:43 +0000 Subject: [PATCH 1/2] prepare worker: Catch unexpected unwinds --- node/core/pvf/src/error.rs | 7 ++++-- node/core/pvf/src/prepare/worker.rs | 34 ++++++++++++++++++++++------- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/node/core/pvf/src/error.rs b/node/core/pvf/src/error.rs index 197573d1d073..8834d3c51738 100644 --- a/node/core/pvf/src/error.rs +++ b/node/core/pvf/src/error.rs @@ -26,6 +26,8 @@ pub enum PrepareError { Prevalidation(String), /// Compilation failed for the given PVF. Preparation(String), + /// An unexpected panic has occured in the preparation worker. + Panic(String), /// Failed to prepare the PVF due to the time limit. TimedOut, /// This state indicates that the process assigned to prepare the artifact wasn't responsible @@ -77,8 +79,9 @@ pub enum InvalidCandidate { impl From for ValidationError { fn from(error: PrepareError) -> Self { let error_str = match error { - PrepareError::Prevalidation(err) => err, - PrepareError::Preparation(err) => err, + PrepareError::Prevalidation(err) => format!("prevalidation: {}", err), + PrepareError::Preparation(err) => format!("preparation: {}", err), + PrepareError::Panic(err) => format!("panic: {}", err), PrepareError::TimedOut => "preparation timeout".to_owned(), PrepareError::DidNotMakeIt => "communication error".to_owned(), }; diff --git a/node/core/pvf/src/prepare/worker.rs b/node/core/pvf/src/prepare/worker.rs index 768bcd4d093c..6bef5e811391 100644 --- a/node/core/pvf/src/prepare/worker.rs +++ b/node/core/pvf/src/prepare/worker.rs @@ -30,7 +30,7 @@ use async_std::{ }; use parity_scale_codec::{Decode, Encode}; use sp_core::hexdisplay::HexDisplay; -use std::{sync::Arc, time::Duration}; +use std::{any::Any, panic, sync::Arc, time::Duration}; const NICENESS_BACKGROUND: i32 = 10; const NICENESS_FOREGROUND: i32 = 0; @@ -318,13 +318,31 @@ pub fn worker_entrypoint(socket_path: &str) { } fn prepare_artifact(code: &[u8]) -> Result { - let blob = match crate::executor_intf::prevalidate(code) { - Err(err) => return Err(PrepareError::Prevalidation(format!("{:?}", err))), - Ok(b) => b, - }; + panic::catch_unwind(|| { + let blob = match crate::executor_intf::prevalidate(code) { + Err(err) => return Err(PrepareError::Prevalidation(format!("{:?}", err))), + Ok(b) => b, + }; + + match crate::executor_intf::prepare(blob) { + Ok(compiled_artifact) => Ok(CompiledArtifact::new(compiled_artifact)), + Err(err) => Err(PrepareError::Preparation(format!("{:?}", err))), + } + }) + .map_err(|panic_payload| PrepareError::Panic(stringify_panic_payload(panic_payload))) + .and_then(|inner_result| inner_result) +} - match crate::executor_intf::prepare(blob) { - Ok(compiled_artifact) => Ok(CompiledArtifact::new(compiled_artifact)), - Err(err) => Err(PrepareError::Preparation(format!("{:?}", err))), +/// Attempt to convert an opaque panic payload to a string. +/// +/// This is a best effort, and is not guaranteed to provide the most accurate value. +fn stringify_panic_payload(payload: Box) -> String { + match payload.downcast::<&'static str>() { + Ok(msg) => msg.to_string(), + Err(payload) => match payload.downcast::() { + Ok(msg) => *msg, + // At least we tried... + Err(_) => "Box".to_string(), + }, } } From f718f05ec75e9b0e65b5ed1258b1b860facce95c Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Thu, 18 Nov 2021 12:11:42 +0000 Subject: [PATCH 2/2] Use more specific wording for unknown panic payload --- node/core/pvf/src/prepare/worker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/core/pvf/src/prepare/worker.rs b/node/core/pvf/src/prepare/worker.rs index 6bef5e811391..57409d9e05b3 100644 --- a/node/core/pvf/src/prepare/worker.rs +++ b/node/core/pvf/src/prepare/worker.rs @@ -342,7 +342,7 @@ fn stringify_panic_payload(payload: Box) -> String { Err(payload) => match payload.downcast::() { Ok(msg) => *msg, // At least we tried... - Err(_) => "Box".to_string(), + Err(_) => "unkown panic payload".to_string(), }, } }