Skip to content

Commit 48e297d

Browse files
joehoylejoehoyle
committed
Refactor how isolates are stored
1 parent 93bbfd0 commit 48e297d

File tree

2 files changed

+115
-59
lines changed

2 files changed

+115
-59
lines changed

‎src/lib.rs‎

Lines changed: 59 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,17 @@ use ext_php_rs::binary::Binary;
22
use ext_php_rs::builders::ClassBuilder;
33
use ext_php_rs::convert::{FromZval, IntoZval};
44
use ext_php_rs::flags::DataType;
5-
use ext_php_rs::types::{ZendHashTable, ZendObject, Zval, ZendClassObject};
5+
use ext_php_rs::types::{ZendClassObject, ZendHashTable, Zval};
66
use ext_php_rs::zend::{ClassEntry, ModuleEntry};
77
use ext_php_rs::{exception::PhpException, zend::ce};
8-
use ext_php_rs::{info_table_end, info_table_row, info_table_start, prelude::*, php_print};
8+
use ext_php_rs::{info_table_end, info_table_row, info_table_start, php_print, prelude::*};
99

1010
use std::collections::HashMap;
1111

1212
mod runtime;
1313

14-
pub use crate::runtime::JSRuntime;
1514
pub use crate::runtime::Error as RuntimeError;
15+
pub use crate::runtime::JSRuntime;
1616

1717
static mut V8JS_TIME_LIMIT_EXCEPTION: Option<&'static ClassEntry> = None;
1818
static mut V8JS_MEMORY_LIMIT_EXCEPTION: Option<&'static ClassEntry> = None;
@@ -54,8 +54,10 @@ pub fn zval_from_jsvalue(result: v8::Local<v8::Value>, scope: &mut v8::HandleSco
5454
}
5555
if result.is_object() {
5656
let object = v8::Local::<v8::Object>::try_from(result).unwrap();
57-
let properties = object.get_own_property_names(scope, Default::default()).unwrap();
58-
let mut obj = ZendClassObject::new(V8Object{});
57+
let properties = object
58+
.get_own_property_names(scope, Default::default())
59+
.unwrap();
60+
let mut obj = ZendClassObject::new(V8Object {});
5961
let zend_object = obj.get_mut_zend_obj();
6062
for index in 0..properties.length() {
6163
let key = properties.get_index(scope, index).unwrap();
@@ -105,11 +107,15 @@ pub fn js_value_from_zval<'a>(
105107
let mut values: Vec<v8::Local<'_, v8::Value>> = Vec::new();
106108
let mut keys: Vec<v8::Local<'_, v8::Name>> = Vec::new();
107109
for (key, elem) in zend_array.iter() {
108-
keys.push(v8::String::new(scope, key.to_string().as_str()).unwrap().into());
110+
keys.push(
111+
v8::String::new(scope, key.to_string().as_str())
112+
.unwrap()
113+
.into(),
114+
);
109115
values.push(js_value_from_zval(scope, elem));
110116
}
111117

112-
if ! zend_array.has_numerical_keys() {
118+
if !zend_array.has_numerical_keys() {
113119
let null: v8::Local<v8::Value> = v8::null(scope).into();
114120
return v8::Object::with_prototype_and_properties(scope, null, &keys[..], &values[..])
115121
.into();
@@ -152,9 +158,12 @@ impl V8Js {
152158
let mut runtime = JSRuntime::new(snapshot_blob);
153159
let object: v8::Global<v8::Value>;
154160
{
155-
let scope = &mut runtime.handle_scope();
156-
let o: v8::Local<v8::Value> = v8::Object::new(scope).into();
157-
object = v8::Global::new(scope, o);
161+
let context = runtime.global_context();
162+
let mut isolate = runtime.isolate();
163+
let isolate = &mut *isolate;
164+
let mut scope = v8::HandleScope::with_context(isolate, context);
165+
let o: v8::Local<v8::Value> = v8::Object::new(&mut scope).into();
166+
object = v8::Global::new(&mut scope, o);
158167
}
159168
runtime.add_global(global_name.as_str(), object);
160169
runtime.add_global_function("var_dump", php_callback_var_dump);
@@ -170,7 +179,7 @@ impl V8Js {
170179
}
171180

172181
pub fn set_module_loader(&mut self, callable: &Zval) {
173-
let state = self.runtime.get_state();
182+
let state = JSRuntime::state(self.runtime.isolate().as_mut());
174183
let mut state = state.borrow_mut();
175184
state.commonjs_module_loader = Some(callable.shallow_clone());
176185
}
@@ -194,8 +203,11 @@ impl V8Js {
194203
match result {
195204
Ok(result) => match result {
196205
Some(result) => {
197-
let mut scope = &mut self.runtime.handle_scope();
198-
let local = v8::Local::new(scope, result);
206+
let context = self.runtime.global_context();
207+
let mut isolate = self.runtime.isolate();
208+
let isolate = &mut *isolate;
209+
let mut scope = v8::HandleScope::with_context(isolate, context);
210+
let local = v8::Local::new(&mut scope, result);
199211
Ok(zval_from_jsvalue(local, &mut scope))
200212
}
201213
None => {
@@ -204,20 +216,20 @@ impl V8Js {
204216
Ok(zval)
205217
}
206218
},
207-
Err(e) => {
208-
match e {
209-
RuntimeError::ExecutionTimeout => {
210-
Err(PhpException::new("".into(), 0, unsafe{ V8JS_TIME_LIMIT_EXCEPTION.unwrap() } ))
211-
},
212-
RuntimeError::MemoryLimitExceeded => {
213-
Err(PhpException::new("".into(), 0, unsafe{ V8JS_MEMORY_LIMIT_EXCEPTION.unwrap() } ))
214-
},
215-
RuntimeError::ScriptExecutionError(error) => {
216-
Err(PhpException::new(error.message.into(), 0, unsafe{ V8JS_SCRIPT_EXCEPTION.unwrap() } ))
217-
}
218-
_ => Err(PhpException::default(String::from("Unknown error.")))
219+
Err(e) => match e {
220+
RuntimeError::ExecutionTimeout => Err(PhpException::new("".into(), 0, unsafe {
221+
V8JS_TIME_LIMIT_EXCEPTION.unwrap()
222+
})),
223+
RuntimeError::MemoryLimitExceeded => Err(PhpException::new("".into(), 0, unsafe {
224+
V8JS_MEMORY_LIMIT_EXCEPTION.unwrap()
225+
})),
226+
RuntimeError::ScriptExecutionError(error) => {
227+
Err(PhpException::new(error.message.into(), 0, unsafe {
228+
V8JS_SCRIPT_EXCEPTION.unwrap()
229+
}))
219230
}
220-
}
231+
_ => Err(PhpException::default(String::from("Unknown error."))),
232+
},
221233
}
222234
}
223235

@@ -228,7 +240,12 @@ impl V8Js {
228240
Some(global) => global,
229241
None => return (),
230242
};
231-
let mut scope = self.runtime.handle_scope();
243+
244+
let context = self.runtime.global_context();
245+
let mut isolate = self.runtime.isolate();
246+
let isolate = &mut *isolate;
247+
let mut scope = v8::HandleScope::with_context(isolate, context);
248+
232249
let global = v8::Local::new(&mut scope, global);
233250
let global: v8::Local<v8::Object> = v8::Local::<v8::Object>::try_from(global).unwrap();
234251
let property_name = v8::String::new(&mut scope, property).unwrap();
@@ -372,7 +389,7 @@ pub fn php_callback_print(
372389
args: v8::FunctionCallbackArguments,
373390
mut _rv: v8::ReturnValue,
374391
) {
375-
php_print!("{}", args.get(0).to_rust_string_lossy(scope) );
392+
php_print!("{}", args.get(0).to_rust_string_lossy(scope));
376393
}
377394

378395
pub fn php_callback_exit(
@@ -465,7 +482,6 @@ pub extern "C" fn php_module_info(_module: *mut ModuleEntry) {
465482
info_table_end!();
466483
}
467484

468-
469485
#[php_startup]
470486
pub fn startup() {
471487
let ce = ClassBuilder::new("V8JsTimeLimitException")
@@ -487,9 +503,22 @@ pub fn startup() {
487503
unsafe { V8JS_SCRIPT_EXCEPTION.replace(ce) };
488504
}
489505

506+
extern "C" fn request_shutdown(module: i32, _: i32) -> i32 {
507+
runtime::ISOLATES.with(|isolates| {
508+
let mut isolates = isolates.borrow_mut();
509+
// Loop through each item in the isolates vec in reverse order and remove them.
510+
for index in (0..isolates.len()).rev() {
511+
isolates.remove(index);
512+
}
513+
});
514+
module
515+
}
516+
490517
#[php_module]
491518
pub fn get_module(module: ModuleBuilder) -> ModuleBuilder {
492-
module.info_function(php_module_info)
519+
module
520+
.info_function(php_module_info)
521+
.request_shutdown_function(request_shutdown)
493522
}
494523

495524
#[cfg(test)]

‎src/runtime.rs‎

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
use ext_php_rs::types::Zval;
2-
use std::cell::RefCell;
2+
use std::cell::{RefCell, RefMut};
33
use std::collections::HashMap;
44
use std::rc::Rc;
55

6+
thread_local! {
7+
pub static ISOLATES: RefCell<Vec<Rc<RefCell<v8::OwnedIsolate>>>> = RefCell::new(Vec::new());
8+
}
9+
610
pub struct JSRuntime {
7-
isolate: v8::OwnedIsolate,
11+
// V8 Isolates have to be dropped in the reverse-order they were created. This presents a challenge as the PHP library can
12+
// init new isolates and drop them in any order.
13+
isolate: Rc<RefCell<v8::OwnedIsolate>>,
814
}
915

1016
pub struct JsRuntimeState {
@@ -35,7 +41,7 @@ pub enum Error {
3541
}
3642

3743
fn init_v8() {
38-
let platform = v8::new_default_platform(0, false).make_shared();
44+
let platform = v8::new_unprotected_default_platform(0, false).make_shared();
3945
v8::V8::initialize_platform(platform);
4046
v8::V8::initialize();
4147
}
@@ -69,6 +75,12 @@ impl JSRuntime {
6975
commonjs_modules: HashMap::new(),
7076
})));
7177

78+
let isolate = Rc::new(RefCell::new(isolate));
79+
// let isolate = ManuallyDrop::new(isolate);
80+
ISOLATES.with(|isolates| {
81+
let mut isolates = isolates.borrow_mut();
82+
isolates.push(isolate.clone());
83+
});
7284
JSRuntime { isolate }
7385
}
7486

@@ -77,23 +89,23 @@ impl JSRuntime {
7789
s.clone()
7890
}
7991

80-
pub fn get_state(&self) -> &Rc<RefCell<JsRuntimeState>> {
81-
self.isolate.get_slot::<Rc<RefCell<JsRuntimeState>>>().unwrap()
92+
pub fn isolate(&self) -> RefMut<'_, v8::OwnedIsolate> {
93+
self.isolate.borrow_mut()
8294
}
8395

8496
pub fn global_context(&self) -> v8::Global<v8::Context> {
85-
let state = Self::state(&self.isolate);
97+
let state = Self::state(&self.isolate());
8698
let state = state.borrow();
8799
state.global_context.clone()
88100
}
89101

90-
pub fn handle_scope(&mut self) -> v8::HandleScope {
102+
pub fn add_global(&mut self, name: &str, value: v8::Global<v8::Value>) {
91103
let context = self.global_context();
92-
v8::HandleScope::with_context(&mut self.isolate, context)
93-
}
104+
let mut isolate = self.isolate();
105+
let isolate = &mut *isolate;
106+
let mut scope = v8::HandleScope::with_context(isolate, context);
94107

95-
pub fn add_global(&mut self, name: &str, value: v8::Global<v8::Value>) {
96-
let mut scope = self.handle_scope();
108+
// let mut scope = self.handle_scope();
97109
let context = scope.get_current_context();
98110
let global = context.global(&mut scope);
99111

@@ -104,7 +116,10 @@ impl JSRuntime {
104116
}
105117

106118
pub fn get_global(&mut self, name: &str) -> Option<v8::Global<v8::Value>> {
107-
let mut scope = self.handle_scope();
119+
let context = self.global_context();
120+
let mut isolate = self.isolate();
121+
let isolate = &mut *isolate;
122+
let mut scope = v8::HandleScope::with_context(isolate, context);
108123
let context = scope.get_current_context();
109124
let global = context.global(&mut scope);
110125

@@ -115,7 +130,7 @@ impl JSRuntime {
115130
}
116131

117132
pub fn add_callback(&mut self, name: &str, callback: Zval) {
118-
let state = Self::state(&self.isolate);
133+
let state = Self::state(&self.isolate());
119134
let mut state = state.borrow_mut();
120135
state.callbacks.insert(name.into(), callback);
121136
}
@@ -132,7 +147,10 @@ impl JSRuntime {
132147
// let global_scope = context.global(scope);
133148
let function: v8::Global<v8::Value>;
134149
{
135-
let mut scope = self.handle_scope();
150+
let context = self.global_context();
151+
let mut isolate = self.isolate();
152+
let isolate = &mut *isolate;
153+
let mut scope = v8::HandleScope::with_context(isolate, context);
136154
let function_builder: v8::FunctionBuilder<v8::Function> =
137155
v8::FunctionBuilder::new(callback);
138156
let f: v8::Local<v8::Value> = function_builder.build(&mut scope).unwrap().into();
@@ -149,18 +167,21 @@ impl JSRuntime {
149167
time_limit: Option<u64>,
150168
memory_limit: Option<u64>,
151169
) -> Result<Option<v8::Global<v8::Value>>, Error> {
152-
let isolate_handle = self.isolate.thread_safe_handle();
153-
let scope = &mut self.handle_scope();
154-
let code = v8::String::new(scope, code).ok_or(Error::V8Error)?;
170+
let isolate_handle = self.isolate().thread_safe_handle();
171+
let context = self.global_context();
172+
let mut isolate = self.isolate();
173+
let isolate = &mut *isolate;
174+
let mut scope = v8::HandleScope::with_context(isolate, context);
175+
let code = v8::String::new(&mut scope, code).ok_or(Error::V8Error)?;
155176

156177
let resource_name = v8::String::new(
157-
scope,
178+
&mut scope,
158179
identifier.unwrap_or("V8Js::executeString".into()).as_str(),
159180
)
160181
.unwrap();
161-
let source_map_url = v8::String::new(scope, "source_map_url").unwrap();
182+
let source_map_url = v8::String::new(&mut scope, "source_map_url").unwrap();
162183
let script_origin = v8::ScriptOrigin::new(
163-
scope,
184+
&mut scope,
164185
resource_name.into(),
165186
0,
166187
0,
@@ -172,10 +193,10 @@ impl JSRuntime {
172193
false,
173194
);
174195

175-
let try_catch = &mut v8::TryCatch::new(scope);
196+
let try_catch = &mut v8::TryCatch::new(&mut scope);
176197

177-
let script =
178-
v8::Script::compile(try_catch, code, Some(&script_origin)).ok_or(Error::JSRuntimeError)?;
198+
let script = v8::Script::compile(try_catch, code, Some(&script_origin))
199+
.ok_or(Error::JSRuntimeError)?;
179200
let stop_flag = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
180201
let time_limit_hit = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
181202
let memory_limit_hit = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
@@ -255,24 +276,30 @@ impl JSRuntime {
255276
Some(result) => Ok(Some(v8::Global::new(try_catch, result))),
256277
None => {
257278
let exception = try_catch.exception().unwrap();
258-
let exception_string = exception
259-
.to_string(try_catch);
279+
let exception_string = exception.to_string(try_catch);
260280

261281
match exception_string {
262282
Some(exception_string) => {
263283
let exception_string = exception_string.to_rust_string_lossy(try_catch);
264284
let message = try_catch.message().unwrap();
265285
Err(Error::ScriptExecutionError(ScriptExecutionErrorData {
266-
file_name: message.get_script_resource_name(try_catch).unwrap().to_rust_string_lossy(try_catch),
267-
line_number: u64::try_from(message.get_line_number(try_catch).unwrap()).unwrap(),
286+
file_name: message
287+
.get_script_resource_name(try_catch)
288+
.unwrap()
289+
.to_rust_string_lossy(try_catch),
290+
line_number: u64::try_from(message.get_line_number(try_catch).unwrap())
291+
.unwrap(),
268292
start_column: u64::try_from(message.get_start_column()).unwrap(),
269293
end_column: u64::try_from(message.get_end_column()).unwrap(),
270294
trace: "".into(), // todo,
271295
message: exception_string,
272-
source_line: message.get_source_line(try_catch).unwrap().to_rust_string_lossy(try_catch),
296+
source_line: message
297+
.get_source_line(try_catch)
298+
.unwrap()
299+
.to_rust_string_lossy(try_catch),
273300
}))
274301
}
275-
None => Ok(None)
302+
None => Ok(None),
276303
}
277304
}
278305
};

0 commit comments

Comments
 (0)