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.

Sign up for GitHub

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

refactor: avoid use global context in panic hook #833

Merged
merged 1 commit into from
Nov 2, 2023
Merged

refactor: avoid use global context in panic hook #833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 35 additions & 4 deletions kclvm/runtime/src/_kcl_run.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading