unregister ops on plugin close

cli/ops/plugin.rs

@@ -1,23 +1,23 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 
 use crate::state::State;
-use deno_core::plugin_api;
 use deno_core::BufVec;
 use deno_core::ErrBox;
 use deno_core::Op;
 use deno_core::OpAsyncFuture;
 use deno_core::OpId;
 use deno_core::OpRegistry;
 use deno_core::ZeroCopyBuf;
+use deno_core::{plugin_api, OpTable};
 use dlopen::symbor::Library;
 use futures::prelude::*;
 use serde_derive::Deserialize;
 use serde_json::Value;
-use std::path::PathBuf;
 use std::pin::Pin;
 use std::rc::Rc;
 use std::task::Context;
 use std::task::Poll;
+use std::{cell::RefCell, path::PathBuf};
 
 pub fn init(s: &Rc<State>) {
   s.register_op_json_sync("op_open_plugin", op_open_plugin);
@@ -43,7 +43,7 @@ pub fn op_open_plugin(
 
   debug!("Loading Plugin: {:#?}", filename);
   let plugin_lib = Library::open(filename).map(Rc::new)?;
-  let plugin_resource = PluginResource::new(&plugin_lib);
+  let plugin_resource = PluginResource::new(&plugin_lib, &state.op_table);
 
   let rid;
   let deno_plugin_init;
@@ -60,30 +60,50 @@ pub fn op_open_plugin(
     };
   }
 
-  let mut interface = PluginInterface::new(state, &plugin_lib);
+  let mut interface = PluginInterface::new(state, &plugin_lib, rid);
   deno_plugin_init(&mut interface);
 
   Ok(json!(rid))
 }
 
 struct PluginResource {
   lib: Rc<Library>,
+  op_table: Rc<RefCell<OpTable<State>>>,
+  ops: Vec<String>,
 }
 
 impl PluginResource {
-  fn new(lib: &Rc<Library>) -> Self {
-    Self { lib: lib.clone() }
+  fn new(lib: &Rc<Library>, op_table: &Rc<RefCell<OpTable<State>>>) -> Self {
+    Self {
+      lib: lib.clone(),
+      op_table: op_table.clone(),
+      ops: Vec::new(),
+    }
+  }
+}
+
+impl Drop for PluginResource {
+  fn drop(&mut self) {
+    let mut table_lock = self.op_table.borrow_mut();
+    for op in &self.ops {
+      table_lock.unregister_op(&op);
+    }
   }
 }
 
 struct PluginInterface<'a> {
   state: &'a State,
   plugin_lib: &'a Rc<Library>,
+  rid: u32,
 }
 
 impl<'a> PluginInterface<'a> {
-  fn new(state: &'a State, plugin_lib: &'a Rc<Library>) -> Self {
-    Self { state, plugin_lib }
+  fn new(state: &'a State, plugin_lib: &'a Rc<Library>, rid: u32) -> Self {
+    Self {
+      state,
+      plugin_lib,
+      rid,
+    }
   }
 }
 
@@ -99,14 +119,12 @@ impl<'a> plugin_api::Interface for PluginInterface<'a> {
     dispatch_op_fn: Box<plugin_api::DispatchOpFn>,
   ) -> OpId {
     let plugin_lib = self.plugin_lib.clone();
-    self.state.register_op(
+    let plugin_rid = self.rid;
+    let op_rid = self.state.register_op(
       name,
       move |state: Rc<State>, mut zero_copy: BufVec| {
-        // This is a hack that ensures that dispatch_op_fn is dropped first to
-        // prevent segfaults.
-        let _ = &dispatch_op_fn;
-        let _ = &plugin_lib;
-        let mut interface = PluginInterface::new(&state, &plugin_lib);
+        let mut interface =
+          PluginInterface::new(&state, &plugin_lib, plugin_rid);
         let op = dispatch_op_fn(&mut interface, &mut zero_copy);
         match op {
           sync_op @ Op::Sync(..) => sync_op,
@@ -119,7 +137,12 @@ impl<'a> plugin_api::Interface for PluginInterface<'a> {
           _ => unreachable!(),
         }
       },
-    )
+    );
+    let mut resource_table = self.state.resource_table.borrow_mut();
+    let plugin_resource =
+      resource_table.get_mut::<PluginResource>(self.rid).unwrap();
+    plugin_resource.ops.push(name.to_string());
+    op_rid
   }
 }
 

cli/state.rs

@@ -55,7 +55,7 @@ pub struct State {
   pub is_internal: bool,
   pub http_client: RefCell<reqwest::Client>,
   pub resource_table: RefCell<ResourceTable>,
-  pub op_table: RefCell<OpTable<Self>>,
+  pub op_table: Rc<RefCell<OpTable<Self>>>,
 }
 
 impl State {

core/ops.rs

@@ -116,6 +116,10 @@ impl<S: OpRegistry> OpTable<S> {
     self.keys().cloned().zip(0..).collect()
   }
 
+  pub fn unregister_op(&mut self, name: &str) {
+    drop(self.0.remove(name));
+  }
+
   fn op_get_op_catalog(state: Rc<S>, _bufs: BufVec) -> Op {
     let ops = state.get_op_catalog();
     let buf = serde_json::to_vec(&ops).map(Into::into).unwrap();

test_plugin/tests/test.js

@@ -117,6 +117,11 @@ Before: ${preStr}
 After: ${postStr}`,
     );
   }
+
+  const ops = Deno.core.ops();
+  if (ops["testSync"] || ops["testAsync"] || ops["testWrapped"]) {
+    throw new Error("Expected plugin ops to be unregistered: ", ops);
+  }
 }
 
 runTestSync();