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

Commit

Permalink
prepare worker: Catch unexpected unwinds (#4304)
Browse files Browse the repository at this point in the history
* prepare worker: Catch unexpected unwinds

* Use more specific wording for unknown panic payload
  • Loading branch information
pepyakin authored and ordian committed Nov 29, 2021
1 parent aac3683 commit 84e09e8
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 10 deletions.
7 changes: 5 additions & 2 deletions node/core/pvf/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -77,8 +79,9 @@ pub enum InvalidCandidate {
impl From<PrepareError> 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(),
};
Expand Down
34 changes: 26 additions & 8 deletions node/core/pvf/src/prepare/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -318,13 +318,31 @@ pub fn worker_entrypoint(socket_path: &str) {
}

fn prepare_artifact(code: &[u8]) -> Result<CompiledArtifact, PrepareError> {
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<dyn Any + Send + 'static>) -> String {
match payload.downcast::<&'static str>() {
Ok(msg) => msg.to_string(),
Err(payload) => match payload.downcast::<String>() {
Ok(msg) => *msg,
// At least we tried...
Err(_) => "unkown panic payload".to_string(),
},
}
}

0 comments on commit 84e09e8

Please sign in to comment.