Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better model panics #1716

Merged
merged 3 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
62 changes: 60 additions & 2 deletions crates/fj-proc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ mod expand;
mod parse;

use proc_macro::TokenStream;
use syn::{parse_macro_input, FnArg, ItemFn};
use proc_macro2::{Ident, Span};
use syn::{
parse_macro_input, punctuated::Punctuated, token::Paren, Expr, ExprCall,
ExprPath, FnArg, ItemFn, PathArguments, PathSegment, Stmt,
};

/// Define a function-based model.
///
Expand Down Expand Up @@ -91,13 +95,67 @@ pub fn model(_: TokenStream, input: TokenStream) -> TokenStream {

match parse::parse(&item) {
Ok(init) => {
let item = without_param_attrs(item);
let mut item = without_param_attrs(item);

// Yes, all of this is to add `fj::abi::initialize_panic_handling();` to the top of the function.
item.block.stmts.insert(
0,
Stmt::Semi(
Expr::Call(ExprCall {
attrs: vec![],
func: Box::new(Expr::Path(ExprPath {
attrs: vec![],
qself: None,
path: syn::Path {
leading_colon: None,
segments: {
let mut segments = Punctuated::new();

segments.push(PathSegment {
ident: Ident::new(
"fj",
Span::call_site(),
),
arguments: PathArguments::None,
});

segments.push(PathSegment {
ident: Ident::new(
"abi",
Span::call_site(),
),
arguments: PathArguments::None,
});

segments.push(PathSegment {
ident: Ident::new(
"initialize_panic_handling",
Span::call_site(),
),
arguments: PathArguments::None,
});

segments
},
},
})),
paren_token: Paren {
span: Span::call_site(),
},
args: Punctuated::new(),
}),
syn::token::Semi::default(),
),
);
Comment on lines +100 to +149
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm far from an expert in procedural macros and might be wrong about what's happening here, but couldn't this be done much easier using the quote! macro? See expand.rs for examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give it a try tonight. I've still got my experiments to do with collecting a backtrace after all.


let tokens = quote::quote! {
#item
#init

};

eprintln!("TOKENS: {}", tokens);

Comment on lines +157 to +158
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It it intentional that this was left in here? Or just leftover debug output?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover debug output... I thought I got them all...

tokens.into()
}
Err(e) => e.into_compile_error().into(),
Expand Down
94 changes: 84 additions & 10 deletions crates/fj/src/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ mod host;
mod metadata;
mod model;

use std::any::Any;
use std::{any::Any, fmt::Display, panic, sync::Mutex};

pub use self::{
context::Context,
Expand Down Expand Up @@ -118,15 +118,89 @@ pub type ModelMetadataResult =
///
pub const INIT_FUNCTION_NAME: &str = "fj_model_init";

fn on_panic(payload: Box<dyn Any + Send>) -> crate::abi::ffi_safe::String {
let msg: &str =
if let Some(s) = payload.downcast_ref::<std::string::String>() {
s.as_str()
} else if let Some(s) = payload.downcast_ref::<&str>() {
s
// Contains details about a panic that we need to pass back to the application from the panic hook.
struct PanicInfo {
message: Option<String>,
location: Option<Location>,
}

impl Display for PanicInfo {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let message = self
.message
.as_ref()
.map_or("No error given", |message| message.as_str());

write!(f, "\"{}\", ", message)?;

if let Some(location) = self.location.as_ref() {
write!(f, "{}", location)?;
} else {
write!(f, "no location given")?;
}

Ok(())
}
}

struct Location {
file: String,
line: u32,
column: u32,
}

impl Display for Location {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}:{}:{}", self.file, self.line, self.column)
}
}

static LAST_PANIC: Mutex<Option<PanicInfo>> = Mutex::new(None);

/// Capturing panics is something Rust really doesn't want you to do, and as such, they make it convoluted.
/// This sets up all the machinery in the background to pull it off.
///
/// It's okay to call this multiple times.
pub fn initialize_panic_handling() {
panic::set_hook(Box::new(|panic_info| {
let mut last_panic =
LAST_PANIC.lock().expect("Panic queue was poisoned."); // FIXME that can probably overflow the stack.
let message = if let Some(s) =
panic_info.payload().downcast_ref::<std::string::String>()
{
Some(s.as_str())
} else {
"A panic occurred"
};
panic_info.payload().downcast_ref::<&str>().copied()
}
.map(|s| s.to_string());

crate::abi::ffi_safe::String::from(msg.to_string())
let location = panic_info.location().map(|location| Location {
file: location.file().to_string(),
line: location.line(),
column: location.column(),
});

*last_panic = Some(PanicInfo { message, location });
}));
}

fn on_panic(_payload: Box<dyn Any + Send>) -> crate::abi::ffi_safe::String {
// The payload is technically no longer needed, but I left it there just in case a change to `catch_unwind` made
// it useful again.
if let Ok(mut panic_info) = LAST_PANIC.lock() {
if let Some(panic_info) = panic_info.take() {
crate::abi::ffi_safe::String::from(format!(
"Panic in model: {}",
panic_info
))
} else {
crate::abi::ffi_safe::String::from(
"Panic in model: No details were given.".to_string(),
)
}
} else {
crate::abi::ffi_safe::String::from(
"Panic in model, but due to a poisoned panic queue the information could not be collected."
.to_string())
}
}