Skip to content

[WASM] Keep a PyObjectWeakRef to the PyObjectRef being moved to a closure #551

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

Merged
merged 4 commits into from
Mar 9, 2019
Merged

[WASM] Keep a PyObjectWeakRef to the PyObjectRef being moved to a closure #551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
88665e9
Keep a Weak to the py_obj being moved to a closure
coolreader18 Feb 26, 2019
5428154
Merge branch 'master' into store-rcs-in-stored-vm
coolreader18 Mar 2, 2019
6361380
Merge branch 'master' into store-rcs-in-stored-vm
coolreader18 Mar 5, 2019
f1b156d
Merge branch 'master' into store-rcs-in-stored-vm
coolreader18 Mar 9, 2019
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
11 changes: 7 additions & 4 deletions wasm/lib/src/convert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ pub fn py_to_js(vm: &mut VirtualMachine, py_obj: PyObjectRef) -> JsValue {
let wasm_vm = WASMVirtualMachine {
id: wasm_id.clone(),
};
let mut py_obj = Some(py_obj);
let weak_py_obj = wasm_vm.push_held_rc(py_obj).unwrap();

let closure =
move |args: Option<Array>, kwargs: Option<Object>| -> Result<JsValue, JsValue> {
let py_obj = match wasm_vm.assert_valid() {
Ok(_) => py_obj.clone().expect("py_obj to be valid if VM is valid"),
Ok(_) => weak_py_obj
.upgrade()
.expect("weak_py_obj to be valid if VM is valid"),
Err(err) => {
py_obj = None;
return Err(err);
}
};
Expand Down Expand Up @@ -57,7 +59,8 @@ pub fn py_to_js(vm: &mut VirtualMachine, py_obj: PyObjectRef) -> JsValue {
as Box<dyn FnMut(Option<Array>, Option<Object>) -> Result<JsValue, JsValue>>);
let func = closure.as_ref().clone();

// TODO: Come up with a way of managing closure handles
// stores pretty much nothing, it's fine to leak this because if it gets dropped
// the error message is worse
closure.forget();

return func;
Expand Down
27 changes: 26 additions & 1 deletion wasm/lib/src/vm_class.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,16 @@ use std::collections::HashMap;
use std::rc::{Rc, Weak};
use wasm_bindgen::prelude::*;

pub trait HeldRcInner {}

impl<T> HeldRcInner for T {}

pub(crate) struct StoredVirtualMachine {
pub vm: VirtualMachine,
pub scope: ScopeRef,
/// you can put a Rc in here, keep it as a Weak, and it'll be held only for
/// as long as the StoredVM is alive
held_rcs: Vec<Rc<dyn HeldRcInner>>,
}

impl StoredVirtualMachine {
Expand All @@ -27,7 +34,11 @@ impl StoredVirtualMachine {
setup_browser_module(&mut vm);
}
vm.wasm_id = Some(id);
StoredVirtualMachine { vm, scope }
StoredVirtualMachine {
vm,
scope,
held_rcs: vec![],
}
}
}

Expand Down Expand Up @@ -211,6 +222,17 @@ impl WASMVirtualMachine {
STORED_VMS.with(|cell| cell.borrow().contains_key(&self.id))
}

pub(crate) fn push_held_rc<T: HeldRcInner + 'static>(
&self,
rc: Rc<T>,
) -> Result<Weak<T>, JsValue> {
self.with(|stored_vm| {
let weak = Rc::downgrade(&rc);
stored_vm.held_rcs.push(rc);
weak
})
}

pub fn assert_valid(&self) -> Result<(), JsValue> {
if self.valid() {
Ok(())
Expand All @@ -234,6 +256,7 @@ impl WASMVirtualMachine {
move |StoredVirtualMachine {
ref mut vm,
ref mut scope,
..
}| {
let value = convert::js_to_py(vm, value);
scope.locals.set_item(&vm.ctx, &name, value);
Expand All @@ -247,6 +270,7 @@ impl WASMVirtualMachine {
move |StoredVirtualMachine {
ref mut vm,
ref mut scope,
..
}| {
let print_fn: Box<Fn(&mut VirtualMachine, PyFuncArgs) -> PyResult> =
if let Some(selector) = stdout.as_string() {
Expand Down Expand Up @@ -315,6 +339,7 @@ impl WASMVirtualMachine {
|StoredVirtualMachine {
ref mut vm,
ref mut scope,
..
}| {
source.push('\n');
let code =
Expand Down