Skip to content

Commit

Permalink
refactor: avoid use global context in panic hook
Browse files Browse the repository at this point in the history 
Signed-off-by: never <[email protected]>
  • Loading branch information
@NeverRaR
NeverRaR committed Nov 2, 2023
1 parent 22aaecf commit 819414e
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 54 deletions.
39 changes: 35 additions & 4 deletions kclvm/runtime/src/_kcl_run.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,18 @@ type kclvm_int_t = i64;
#[allow(dead_code, non_camel_case_types)]
type kclvm_float_t = f64;

// const SHOULD_PROFILE: bool = false;
#[derive(Debug, Default)]
pub(crate) struct RuntimePanicRecord {
pub kcl_panic_info: bool,
pub message: String,
pub rust_file: String,
pub rust_line: i32,
pub rust_col: i32,
}

thread_local! {
static KCL_RUNTIME_PANIC_RECORD: std::cell::RefCell<RuntimePanicRecord> = std::cell::RefCell::new(RuntimePanicRecord::default())
}

#[no_mangle]
#[runtime_fn]
Expand All @@ -56,9 +67,25 @@ pub unsafe extern "C" fn _kcl_run(

let prev_hook = std::panic::take_hook();
std::panic::set_hook(Box::new(|info: &std::panic::PanicInfo| {
let ctx = Context::current_context_mut();
ctx.set_panic_info(info);
let _ = ctx;
KCL_RUNTIME_PANIC_RECORD.with(|record| {
let mut record = record.borrow_mut();
record.kcl_panic_info = true;

record.message = if let Some(s) = info.payload().downcast_ref::<&str>() {
s.to_string()
} else if let Some(s) = info.payload().downcast_ref::<&String>() {
(*s).clone()
} else if let Some(s) = info.payload().downcast_ref::<String>() {
(*s).clone()
} else {
"".to_string()
};
if let Some(location) = info.location() {
record.rust_file = location.file().to_string();
record.rust_line = location.line() as i32;
record.rust_col = location.column() as i32;
}
})
}));

let result = std::panic::catch_unwind(|| {
Expand All @@ -77,6 +104,10 @@ pub unsafe extern "C" fn _kcl_run(
)
});
std::panic::set_hook(prev_hook);
KCL_RUNTIME_PANIC_RECORD.with(|record| {
let record = record.borrow();
Context::current_context_mut().set_panic_info(&record);
});
match result {
Ok(n) => {
let json_panic_info = Context::current_context().get_panic_info_json_string();
Expand Down
11 changes: 7 additions & 4 deletions kclvm/runtime/src/api/kclvm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ use crate::{new_mut_ptr, IndexMap};
use indexmap::IndexSet;
use serde::{Deserialize, Serialize};
use std::collections::{HashMap, HashSet};
use std::panic::UnwindSafe;
use std::rc::Rc;
use std::{
cell::RefCell,
cmp::Ordering,
hash::{Hash, Hasher},
rc::Rc,
};

#[allow(non_upper_case_globals)]
Expand Down Expand Up @@ -376,8 +377,8 @@ pub struct Context {

pub imported_pkgpath: HashSet<String>,
pub app_args: HashMap<String, u64>,
pub instances: RefCell<HashMap<String, Vec<ValueRef>>>,
pub all_schemas: RefCell<HashMap<String, SchemaType>>,
pub instances: HashMap<String, Vec<ValueRef>>,
pub all_schemas: HashMap<String, SchemaType>,
pub import_names: IndexMap<String, IndexMap<String, String>>,
pub symbol_names: Vec<String>,
pub symbol_values: Vec<Value>,
Expand All @@ -389,6 +390,8 @@ pub struct Context {
pub objects: IndexSet<usize>,
}

impl UnwindSafe for Context {}

#[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize)]
pub struct BacktraceFrame {
pub file: String,
Expand Down Expand Up @@ -421,7 +424,7 @@ impl BacktraceFrame {
impl Context {
pub fn new() -> Self {
Context {
instances: RefCell::new(HashMap::new()),
instances: HashMap::new(),
panic_info: PanicInfo {
kcl_func: "kclvm_main".to_string(),
..Default::default()
Expand Down
27 changes: 7 additions & 20 deletions kclvm/runtime/src/context/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pub mod api;
pub use api::*;
use std::fmt;

use crate::{BacktraceFrame, PanicInfo};
use crate::{BacktraceFrame, PanicInfo, RuntimePanicRecord};

#[allow(non_camel_case_types)]
type kclvm_value_ref_t = crate::ValueRef;
Expand Down Expand Up @@ -186,18 +186,10 @@ impl crate::Context {
self.panic_info.is_warning = true;
}

pub fn set_panic_info(&mut self, info: &std::panic::PanicInfo) {
pub(crate) fn set_panic_info(&mut self, record: &RuntimePanicRecord) {
self.panic_info.__kcl_PanicInfo__ = true;

self.panic_info.message = if let Some(s) = info.payload().downcast_ref::<&str>() {
s.to_string()
} else if let Some(s) = info.payload().downcast_ref::<&String>() {
(*s).clone()
} else if let Some(s) = info.payload().downcast_ref::<String>() {
(*s).clone()
} else {
"".to_string()
};
self.panic_info.message = record.message.clone();
if self.cfg.debug_mode {
self.panic_info.backtrace = self.backtrace.clone();
self.panic_info.backtrace.push(BacktraceFrame {
Expand All @@ -207,15 +199,10 @@ impl crate::Context {
line: self.panic_info.kcl_line,
});
}
if let Some(location) = info.location() {
self.panic_info.rust_file = location.file().to_string();
self.panic_info.rust_line = location.line() as i32;
self.panic_info.rust_col = location.column() as i32;
} else {
self.panic_info.rust_file = "".to_string();
self.panic_info.rust_line = 0;
self.panic_info.rust_col = 0;
}

self.panic_info.rust_file = record.rust_file.clone();
self.panic_info.rust_line = record.rust_line;
self.panic_info.rust_col = record.rust_col;
}
}

Expand Down
17 changes: 7 additions & 10 deletions kclvm/runtime/src/value/api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,16 +272,15 @@ pub unsafe extern "C" fn kclvm_value_schema_with_config(
let schema =
schema_dict.dict_to_schema(name, pkgpath, &config_keys, config_meta, optional_mapping);
// Runtime context
let ctx = Context::current_context();
let ctx = Context::current_context_mut();
if record_instance.is_truthy()
&& (instance_pkgpath.is_empty() || instance_pkgpath == MAIN_PKG_PATH)
{
// Record schema instance in the context
let mut instance_map = ctx.instances.borrow_mut();
if !instance_map.contains_key(&runtime_type) {
instance_map.insert(runtime_type.clone(), vec![]);
if !ctx.instances.contains_key(&runtime_type) {
ctx.instances.insert(runtime_type.clone(), vec![]);
}
instance_map
ctx.instances
.get_mut(&runtime_type)
.unwrap()
.push(schema_dict.clone());
Expand Down Expand Up @@ -377,7 +376,6 @@ pub unsafe extern "C" fn kclvm_value_schema_function(
let attr_map = ptr_as_ref(attr_map);
let attr_dict = attr_map.as_dict_ref();
let ctx = Context::current_context_mut();
let mut all_schemas = ctx.all_schemas.borrow_mut();
let schema_ty = SchemaType {
name: runtime_type.to_string(),
attrs: attr_dict
Expand All @@ -388,7 +386,7 @@ pub unsafe extern "C" fn kclvm_value_schema_function(
has_index_signature: attr_dict.attr_map.contains_key(CAL_MAP_INDEX_SIGNATURE),
func: schema_func.clone(),
};
all_schemas.insert(runtime_type.to_string(), schema_ty);
ctx.all_schemas.insert(runtime_type.to_string(), schema_ty);
new_mut_ptr(schema_func)
}

Expand Down Expand Up @@ -2000,10 +1998,9 @@ pub unsafe extern "C" fn kclvm_schema_instances(
true
};
let runtime_type = &function.runtime_type;
let instance_map = ctx.instances.borrow_mut();
if instance_map.contains_key(runtime_type) {
if ctx.instances.contains_key(runtime_type) {
let mut list = ValueRef::list(None);
for v in instance_map.get(runtime_type).unwrap() {
for v in ctx.instances.get(runtime_type).unwrap() {
if v.is_schema() {
let schema = v.as_schema();
if main_pkg {
Expand Down
20 changes: 4 additions & 16 deletions kclvm/runtime/src/value/val_type.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,9 @@ pub fn resolve_schema(schema: &ValueRef, keys: &[String]) -> ValueRef {
let schema_type_name = schema_runtime_type(&schema_value.name, &schema_value.pkgpath);
let ctx = Context::current_context_mut();
let now_meta_info = ctx.panic_info.clone();
let has_schema_type = {
let all_schemas = ctx.all_schemas.borrow_mut();
all_schemas.contains_key(&schema_type_name)
};
let has_schema_type = { ctx.all_schemas.contains_key(&schema_type_name) };
if has_schema_type {
let schema_type = {
let all_schemas = ctx.all_schemas.borrow_mut();
all_schemas.get(&schema_type_name).unwrap().clone()
};
let schema_type = { ctx.all_schemas.get(&schema_type_name).unwrap().clone() };
let schema_type = schema_type.func.as_function();
let schema_fn_ptr = schema_type.fn_ptr;
let keys = keys.iter().map(|v| v.as_str()).collect();
Expand Down Expand Up @@ -276,15 +270,9 @@ pub fn convert_collection_value(value: &ValueRef, tpe: &str) -> ValueRef {
}
}
}
let has_schema_type = {
let all_schemas = ctx.all_schemas.borrow_mut();
all_schemas.contains_key(&schema_type_name)
};
let has_schema_type = { ctx.all_schemas.contains_key(&schema_type_name) };
if has_schema_type {
let schema_type = {
let all_schemas = ctx.all_schemas.borrow_mut();
all_schemas.get(&schema_type_name).unwrap().clone()
};
let schema_type = { ctx.all_schemas.get(&schema_type_name).unwrap().clone() };
let schema_fn = schema_type.func.as_function();
let schema_fn_ptr = schema_fn.fn_ptr;
let value = unsafe {
Expand Down

0 comments on commit 819414e

Please sign in to comment.