Scopes refactor #406
Scopes refactor #406
Conversation
This makes it possible to add a run-time check that verifies that the specified closure is actually the one that contains the local handle.
There was a problem hiding this comment.
LGTM
I can't say I understand very well what's happening here. It's very complex - but I have zero doubt that it's justified, given how much energy you've poured into this.
I just request some documentation, somewhere, about how this is all organized. This is going to take many days for another engineer to get up to speed on what's happening, and why. A couple of sentences of docs probably would go a long way.
This makes it possible to add a run-time check that verifies that the specified closure is actually the one that contains the local handle.
This makes it possible to add a run-time check that verifies that the specified closure is actually the one that contains the local handle.
|
current Deno delta: denoland/deno@master...piscisaureus:scopes_refactor_update |
|
@piscisaureus Very nice to see the deno delta - thanks. One thing that sticks out is that there are several new instances of unsafe. - let mut cbs = v8::CallbackScope::new_escapable(context);
- let mut hs = v8::EscapableHandleScope::new(cbs.enter());
- let scope = hs.enter();
+ let scope = &mut unsafe { v8::CallbackScope::new(context) };It would be nice to have some sort of utility function in rusty_v8 to do that safely (maybe impl From?) |
I think might be hard to get right -- everytime we're entering the vm we'd have to set some flag that says "callbacks possible - CallbackScope ok!", and then we have to clear that flag as we exit the VM. But who knows which callback could be triggered when... I think the long term solution is what we are already doing for FunctionCallbackInfo/PropertyCallbackInfo, that is, rusty_v8 takes care of creating a scope and passes a safe one to the user callback. here for instance - there's no unsafe and not even a CallbackScope. |
| @@ -61,6 +61,10 @@ fn build_v8() { | |||
| vec!["is_debug=false".to_string()] | |||
| }; | |||
|
|
|||
| if !cargo_gn::is_debug() { | |||
| gn_args.push("v8_enable_handle_zapping=false".to_string()); | |||
| } | |||
There was a problem hiding this comment.
Can you add a comment here? What's this for?
piscisaureus
force-pushed
the
scope
branch
5 times, most recently
from
7e9a8eb to
464602e
Compare
Local handles never need to be mutable. This patch also rounds up the last few places where we were still asking the user to pass an `&mut T` to an API method.
No description provided.