Skip to content

Commit

Permalink
fix(napi): allow cleanup hook to remove itself (denoland#17402)
Browse files Browse the repository at this point in the history
This commit fixes "cleanup hooks" in NAPI integration in two ways:
- don't hold to RefCell's borrow while iterating over hooks
- allow a hook to remove itself when being called
  • Loading branch information
bartlomieju committed Jan 13, 2023
1 parent d4767a1 commit 2251141
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 4 deletions.
29 changes: 25 additions & 4 deletions ext/napi/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,11 +343,32 @@ pub struct NapiState {

impl Drop for NapiState {
fn drop(&mut self) {
let mut hooks = self.env_cleanup_hooks.borrow_mut();
let hooks = {
let h = self.env_cleanup_hooks.borrow_mut();
h.clone()
};

// Hooks are supposed to be run in LIFO order
let hooks = hooks.drain(..).rev();
for (fn_ptr, data) in hooks {
(fn_ptr)(data);
let hooks_to_run = hooks.into_iter().rev();

for hook in hooks_to_run {
// This hook might have been removed by a previous hook, in such case skip it here.
if !self
.env_cleanup_hooks
.borrow()
.iter()
.any(|pair| pair.0 == hook.0 && pair.1 == hook.1)
{
continue;
}

(hook.0)(hook.1);
{
self
.env_cleanup_hooks
.borrow_mut()
.retain(|pair| !(pair.0 == hook.0 && pair.1 == hook.1));
}
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions test_napi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ extern "C" fn cleanup(arg: *mut c_void) {
println!("cleanup({})", arg as i64);
}

extern "C" fn remove_this_hook(arg: *mut c_void) {
let env = arg as napi_env;
unsafe { napi_remove_env_cleanup_hook(env, Some(remove_this_hook), arg) };
}

static SECRET: i64 = 42;
static WRONG_SECRET: i64 = 17;
static THIRD_SECRET: i64 = 18;
Expand All @@ -81,6 +86,7 @@ extern "C" fn install_cleanup_hook(
napi_add_env_cleanup_hook(env, Some(cleanup), WRONG_SECRET as *mut c_void);
napi_add_env_cleanup_hook(env, Some(cleanup), SECRET as *mut c_void);
napi_add_env_cleanup_hook(env, Some(cleanup), THIRD_SECRET as *mut c_void);
napi_add_env_cleanup_hook(env, Some(remove_this_hook), env as *mut c_void);
napi_remove_env_cleanup_hook(
env,
Some(cleanup),
Expand Down

0 comments on commit 2251141

Please sign in to comment.