Skip to content

Commit 64b642b

Browse files
committed
make safe context lifetime
1 parent 4df213c commit 64b642b

File tree

16 files changed

+287
-305
lines changed

16 files changed

+287
-305
lines changed

packages/cubejs-backend-native/src/node_export.rs

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::stream::OnDrainHandler;
2323
use crate::tokio_runtime_node;
2424
use crate::transport::NodeBridgeTransport;
2525
use crate::utils::batch_to_rows;
26-
use cubenativeutils::wrappers::neon::context::ContextHolder;
26+
use cubenativeutils::wrappers::neon::context::neon_run_with_guarded_lifetime;
2727
use cubenativeutils::wrappers::neon::inner_types::NeonInnerTypes;
2828
use cubenativeutils::wrappers::neon::object::NeonObject;
2929
use cubenativeutils::wrappers::object_handle::NativeObjectHandle;
@@ -463,38 +463,29 @@ pub fn setup_logger(mut cx: FunctionContext) -> JsResult<JsUndefined> {
463463
//============ sql planner ===================
464464

465465
fn build_sql_and_params(cx: FunctionContext) -> JsResult<JsValue> {
466-
//IMPORTANT It seems to be safe here, because context lifetime is bound to function, but this
467-
//context should be used only inside function
468-
let mut cx = extend_function_context_lifetime(cx);
469-
let options = cx.argument::<JsValue>(0)?;
470-
let options_tmp = options.clone();
471-
472-
let neon_context_holder = ContextHolder::new(cx);
473-
474-
let options = NativeObjectHandle::<NeonInnerTypes<'static, FunctionContext<'static>>>::new(
475-
NeonObject::new(neon_context_holder.clone(), options),
476-
);
477-
478-
let context_holder =
479-
NativeContextHolder::<NeonInnerTypes<'static, FunctionContext<'static>>>::new(
466+
neon_run_with_guarded_lifetime(cx, |neon_context_holder| {
467+
let options =
468+
NativeObjectHandle::<NeonInnerTypes<FunctionContext<'static>>>::new(NeonObject::new(
469+
neon_context_holder.clone(),
470+
neon_context_holder
471+
.with_context(|cx| cx.argument::<JsValue>(0))
472+
.unwrap()?,
473+
));
474+
475+
let context_holder = NativeContextHolder::<NeonInnerTypes<FunctionContext<'static>>>::new(
480476
neon_context_holder,
481477
);
482478

483-
let base_query_options = Rc::new(NativeBaseQueryOptions::from_native(options).unwrap());
479+
let base_query_options = Rc::new(NativeBaseQueryOptions::from_native(options).unwrap());
484480

485-
let base_query = BaseQuery::try_new(context_holder.clone(), base_query_options).unwrap();
481+
let base_query = BaseQuery::try_new(context_holder.clone(), base_query_options).unwrap();
486482

487-
//arg_clrep.into_js(&mut cx)
488-
let res = base_query.build_sql_and_params();
489-
490-
let result: NeonObject<'static, FunctionContext<'static>> = res.into_object();
491-
let result = result.into_object();
492-
493-
Ok(result)
494-
}
483+
let res = base_query.build_sql_and_params();
495484

496-
fn extend_function_context_lifetime<'a>(cx: FunctionContext<'a>) -> FunctionContext<'static> {
497-
unsafe { std::mem::transmute::<FunctionContext<'a>, FunctionContext<'static>>(cx) }
485+
let result: NeonObject<FunctionContext<'static>> = res.into_object();
486+
let result = result.into_object();
487+
Ok(result)
488+
})
498489
}
499490

500491
fn debug_js_to_clrepr_to_js(mut cx: FunctionContext) -> JsResult<JsValue> {
Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
use super::inner_types::InnerTypes;
22
use super::object::NativeBox;
33
use super::object_handle::NativeObjectHandle;
4+
use cubesql::CubeError;
45

56
pub trait NativeContext<IT: InnerTypes>: Clone {
6-
fn boolean(&self, v: bool) -> IT::Boolean;
7-
fn string(&self, v: String) -> IT::String;
8-
fn number(&self, v: f64) -> IT::Number;
9-
fn undefined(&self) -> NativeObjectHandle<IT>;
10-
fn empty_array(&self) -> IT::Array;
11-
fn empty_struct(&self) -> IT::Struct;
7+
fn boolean(&self, v: bool) -> Result<IT::Boolean, CubeError>;
8+
fn string(&self, v: String) -> Result<IT::String, CubeError>;
9+
fn number(&self, v: f64) -> Result<IT::Number, CubeError>;
10+
fn undefined(&self) -> Result<NativeObjectHandle<IT>, CubeError>;
11+
fn empty_array(&self) -> Result<IT::Array, CubeError>;
12+
fn empty_struct(&self) -> Result<IT::Struct, CubeError>;
1213
//fn boxed<T: 'static>(&self, value: T) -> impl NativeBox<IT, T>;
13-
fn to_string_fn(&self, result: String) -> IT::Function;
14+
fn to_string_fn(&self, result: String) -> Result<IT::Function, CubeError>;
1415
}
1516

1617
#[derive(Clone)]
@@ -25,26 +26,26 @@ impl<IT: InnerTypes> NativeContextHolder<IT> {
2526
pub fn context(&self) -> &impl NativeContext<IT> {
2627
&self.context
2728
}
28-
pub fn boolean(&self, v: bool) -> IT::Boolean {
29+
pub fn boolean(&self, v: bool) -> Result<IT::Boolean, CubeError> {
2930
self.context.boolean(v)
3031
}
31-
pub fn string(&self, v: String) -> IT::String {
32+
pub fn string(&self, v: String) -> Result<IT::String, CubeError> {
3233
self.context.string(v)
3334
}
34-
pub fn number(&self, v: f64) -> IT::Number {
35+
pub fn number(&self, v: f64) -> Result<IT::Number, CubeError> {
3536
self.context.number(v)
3637
}
37-
pub fn undefined(&self) -> NativeObjectHandle<IT> {
38+
pub fn undefined(&self) -> Result<NativeObjectHandle<IT>, CubeError> {
3839
self.context.undefined()
3940
}
40-
pub fn empty_array(&self) -> IT::Array {
41+
pub fn empty_array(&self) -> Result<IT::Array, CubeError> {
4142
self.context.empty_array()
4243
}
43-
pub fn empty_struct(&self) -> IT::Struct {
44+
pub fn empty_struct(&self) -> Result<IT::Struct, CubeError> {
4445
self.context.empty_struct()
4546
}
4647
#[allow(dead_code)]
47-
pub fn to_string_fn(&self, result: String) -> IT::Function {
48+
pub fn to_string_fn(&self, result: String) -> Result<IT::Function, CubeError> {
4849
self.context.to_string_fn(result)
4950
}
5051
}

rust/cubenativeutils/src/wrappers/neon/context.rs

Lines changed: 77 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,29 @@ use crate::wrappers::object::NativeObject;
1010
use crate::wrappers::object_handle::NativeObjectHandle;
1111
use cubesql::CubeError;
1212
use neon::prelude::*;
13-
use std::cell::{RefCell, RefMut};
14-
use std::marker::PhantomData;
13+
use std::cell::RefCell;
14+
use std::panic::{catch_unwind, resume_unwind, AssertUnwindSafe};
1515
use std::rc::{Rc, Weak};
16-
pub struct ContextWrapper<'cx, C: Context<'cx>> {
16+
17+
pub trait NoenContextLifetimeExpand<'cx> {
18+
type ExpandedResult: Context<'static>;
19+
fn expand_lifetime(self) -> Self::ExpandedResult;
20+
}
21+
22+
impl<'cx> NoenContextLifetimeExpand<'cx> for FunctionContext<'cx> {
23+
type ExpandedResult = FunctionContext<'static>;
24+
fn expand_lifetime(self) -> Self::ExpandedResult {
25+
unsafe { std::mem::transmute::<FunctionContext<'cx>, FunctionContext<'static>>(self) }
26+
}
27+
}
28+
29+
pub struct ContextWrapper<C: Context<'static>> {
1730
cx: C,
18-
lifetime: PhantomData<&'cx ()>,
1931
}
2032

21-
impl<'cx, C: Context<'cx>> ContextWrapper<'cx, C> {
33+
impl<C: Context<'static>> ContextWrapper<C> {
2234
pub fn new(cx: C) -> Rc<RefCell<Self>> {
23-
Rc::new(RefCell::new(Self {
24-
cx,
25-
lifetime: Default::default(),
26-
}))
35+
Rc::new(RefCell::new(Self { cx }))
2736
}
2837

2938
pub fn with_context<T, F>(&mut self, f: F) -> T
@@ -38,125 +47,105 @@ impl<'cx, C: Context<'cx>> ContextWrapper<'cx, C> {
3847
}
3948
}
4049

41-
pub struct ContextHolder<'cx, C: Context<'cx>> {
42-
context: Rc<RefCell<ContextWrapper<'cx, C>>>,
50+
pub fn neon_run_with_guarded_lifetime<'cx, C, T, F>(cx: C, func: F) -> T
51+
where
52+
C: Context<'cx> + NoenContextLifetimeExpand<'cx>,
53+
F: FnOnce(ContextHolder<C::ExpandedResult>) -> T,
54+
{
55+
let context = ContextWrapper::new(cx.expand_lifetime());
56+
let context_holder = ContextHolder::new(Rc::downgrade(&context));
57+
let res = catch_unwind(AssertUnwindSafe(|| func(context_holder)));
58+
match Rc::try_unwrap(context) {
59+
Ok(_) => {}
60+
Err(_) => panic!("Guarded context have more then one reference"),
61+
};
62+
63+
match res {
64+
Ok(res) => res,
65+
Err(e) => resume_unwind(e),
66+
}
4367
}
4468

45-
impl<'cx, C: Context<'cx> + 'cx> ContextHolder<'cx, C> {
46-
pub fn new(cx: C) -> Self {
47-
Self {
48-
context: ContextWrapper::new(cx),
49-
}
50-
}
69+
pub struct ContextHolder<C: Context<'static>> {
70+
context: Weak<RefCell<ContextWrapper<C>>>,
71+
}
5172

52-
pub fn borrow_mut(&self) -> RefMut<ContextWrapper<'cx, C>> {
53-
self.context.borrow_mut()
73+
impl<C: Context<'static>> ContextHolder<C> {
74+
fn new(context: Weak<RefCell<ContextWrapper<C>>>) -> Self {
75+
Self { context }
5476
}
5577

56-
pub fn with_context<T, F>(&self, f: F) -> T
78+
pub fn with_context<T, F>(&self, f: F) -> Result<T, CubeError>
5779
where
5880
F: FnOnce(&mut C) -> T,
5981
{
60-
let mut context = self.context.borrow_mut();
61-
context.with_context(f)
62-
}
63-
64-
pub fn weak(&self) -> WeakContextHolder<'cx, C> {
65-
WeakContextHolder {
66-
context: Rc::downgrade(&self.context),
82+
if let Some(context) = self.context.upgrade() {
83+
let mut cx = context.borrow_mut();
84+
let res = cx.with_context(f);
85+
Ok(res)
86+
} else {
87+
Err(CubeError::internal(format!(
88+
"Call to neon context outside of its lifetime"
89+
)))
6790
}
6891
}
6992
}
7093

71-
impl<'cx, C: Context<'cx> + 'cx> NativeContext<NeonInnerTypes<'cx, C>> for ContextHolder<'cx, C> {
72-
fn boolean(&self, v: bool) -> NeonBoolean<'cx, C> {
73-
let obj = NeonObject::new(self.clone(), self.with_context(|cx| cx.boolean(v).upcast()));
74-
obj.into_boolean().unwrap()
94+
impl<C: Context<'static> + 'static> NativeContext<NeonInnerTypes<C>> for ContextHolder<C> {
95+
fn boolean(&self, v: bool) -> Result<NeonBoolean<C>, CubeError> {
96+
let obj = NeonObject::new(
97+
self.clone(),
98+
self.with_context(|cx| cx.boolean(v).upcast())?,
99+
);
100+
obj.into_boolean()
75101
}
76102

77-
fn string(&self, v: String) -> NeonString<'cx, C> {
78-
let obj = NeonObject::new(self.clone(), self.with_context(|cx| cx.string(v).upcast()));
79-
obj.into_string().unwrap()
103+
fn string(&self, v: String) -> Result<NeonString<C>, CubeError> {
104+
let obj = NeonObject::new(self.clone(), self.with_context(|cx| cx.string(v).upcast())?);
105+
obj.into_string()
80106
}
81107

82-
fn number(&self, v: f64) -> NeonNumber<'cx, C> {
83-
let obj = NeonObject::new(self.clone(), self.with_context(|cx| cx.number(v).upcast()));
84-
obj.into_number().unwrap()
108+
fn number(&self, v: f64) -> Result<NeonNumber<C>, CubeError> {
109+
let obj = NeonObject::new(self.clone(), self.with_context(|cx| cx.number(v).upcast())?);
110+
obj.into_number()
85111
}
86112

87-
fn undefined(&self) -> NativeObjectHandle<NeonInnerTypes<'cx, C>> {
88-
NativeObjectHandle::new(NeonObject::new(
113+
fn undefined(&self) -> Result<NativeObjectHandle<NeonInnerTypes<C>>, CubeError> {
114+
Ok(NativeObjectHandle::new(NeonObject::new(
89115
self.clone(),
90-
self.with_context(|cx| cx.undefined().upcast()),
91-
))
116+
self.with_context(|cx| cx.undefined().upcast())?,
117+
)))
92118
}
93119

94-
fn empty_array(&self) -> NeonArray<'cx, C> {
120+
fn empty_array(&self) -> Result<NeonArray<C>, CubeError> {
95121
let obj = NeonObject::new(
96122
self.clone(),
97-
self.with_context(|cx| cx.empty_array().upcast()),
123+
self.with_context(|cx| cx.empty_array().upcast())?,
98124
);
99-
obj.into_array().unwrap()
125+
obj.into_array()
100126
}
101127

102-
fn empty_struct(&self) -> NeonStruct<'cx, C> {
128+
fn empty_struct(&self) -> Result<NeonStruct<C>, CubeError> {
103129
let obj = NeonObject::new(
104130
self.clone(),
105-
self.with_context(|cx| cx.empty_object().upcast()),
131+
self.with_context(|cx| cx.empty_object().upcast())?,
106132
);
107-
obj.into_struct().unwrap()
133+
obj.into_struct()
108134
}
109-
fn to_string_fn(&self, result: String) -> NeonFunction<'cx, C> {
135+
fn to_string_fn(&self, result: String) -> Result<NeonFunction<C>, CubeError> {
110136
let obj = NeonObject::new(
111137
self.clone(),
112138
self.with_context(|cx| {
113139
JsFunction::new(cx, move |mut c| Ok(c.string(result.clone())))
114140
.unwrap()
115141
.upcast()
116-
}),
142+
})?,
117143
);
118-
obj.into_function().unwrap()
119-
}
120-
}
121-
122-
impl<'cx, C: Context<'cx>> Clone for ContextHolder<'cx, C> {
123-
fn clone(&self) -> Self {
124-
Self {
125-
context: self.context.clone(),
126-
}
127-
}
128-
}
129-
130-
pub struct WeakContextHolder<'cx, C: Context<'cx>> {
131-
context: Weak<RefCell<ContextWrapper<'cx, C>>>,
132-
}
133-
134-
impl<'cx, C: Context<'cx>> WeakContextHolder<'cx, C> {
135-
pub fn try_upgrade<'a>(&'a self) -> Result<ContextHolder<'a, C>, CubeError>
136-
where
137-
'a: 'cx,
138-
{
139-
if let Some(context) = self.context.upgrade() {
140-
Ok(ContextHolder { context })
141-
} else {
142-
Err(CubeError::internal(format!("Neon context is not alive")))
143-
}
144-
}
145-
pub fn with_context<T, F>(&self, f: F) -> Result<T, CubeError>
146-
where
147-
F: FnOnce(&mut C) -> T,
148-
{
149-
if let Some(context) = self.context.upgrade() {
150-
let mut cx = context.borrow_mut();
151-
let res = cx.with_context(f);
152-
Ok(res)
153-
} else {
154-
Err(CubeError::internal(format!("Neon context is not alive")))
155-
}
144+
obj.into_function()
156145
}
157146
}
158147

159-
impl<'cx, C: Context<'cx>> Clone for WeakContextHolder<'cx, C> {
148+
impl<C: Context<'static>> Clone for ContextHolder<C> {
160149
fn clone(&self) -> Self {
161150
Self {
162151
context: self.context.clone(),

rust/cubenativeutils/src/wrappers/neon/inner_types.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,25 @@ use crate::wrappers::inner_types::InnerTypes;
88
use neon::prelude::*;
99
use std::marker::PhantomData;
1010

11-
pub struct NeonInnerTypes<'cx: 'static, C: Context<'cx>> {
12-
lifetime: PhantomData<&'cx ContextHolder<'cx, C>>,
11+
pub struct NeonInnerTypes<C: Context<'static>> {
12+
marker: PhantomData<ContextHolder<C>>,
1313
}
1414

15-
impl<'cx, C: Context<'cx>> Clone for NeonInnerTypes<'cx, C> {
15+
impl<C: Context<'static>> Clone for NeonInnerTypes<C> {
1616
fn clone(&self) -> Self {
1717
Self {
18-
lifetime: Default::default(),
18+
marker: Default::default(),
1919
}
2020
}
2121
}
2222

23-
impl<'cx: 'static, C: Context<'cx>> InnerTypes for NeonInnerTypes<'cx, C> {
24-
type Object = NeonObject<'cx, C>;
25-
type Context = ContextHolder<'cx, C>;
26-
type Array = NeonArray<'cx, C>;
27-
type Struct = NeonStruct<'cx, C>;
28-
type String = NeonString<'cx, C>;
29-
type Boolean = NeonBoolean<'cx, C>;
30-
type Function = NeonFunction<'cx, C>;
31-
type Number = NeonNumber<'cx, C>;
23+
impl<C: Context<'static> + 'static> InnerTypes for NeonInnerTypes<C> {
24+
type Object = NeonObject<C>;
25+
type Context = ContextHolder<C>;
26+
type Array = NeonArray<C>;
27+
type Struct = NeonStruct<C>;
28+
type String = NeonString<C>;
29+
type Boolean = NeonBoolean<C>;
30+
type Function = NeonFunction<C>;
31+
type Number = NeonNumber<C>;
3232
}

0 commit comments

Comments
 (0)