Deno: Deno.close() does not work with plugins

[Pandawan]

I know plugins are currently unstable so this might be something that is in the works, but I thought I'd report it anyways.

It seems calling Deno.close() on a previously opened Deno plugin (with Deno.openPlugin()) does not actually do anything. The ops registered by the plugin are still available in Deno.core.ops() and they are still callable.

I've written a quick test script to reproduce the issue but you need your own plugin

import { resolve } from "https://deno.land/[email protected]/path/mod.ts";
import { assert } from "https://deno.land/[email protected]/testing/asserts.ts";

// Pass in the path to a plugin here
const pluginPath = Deno.args[0] || './target/debug/libdeno_notify.dylib';
const filename = resolve(pluginPath);
console.log("Opening plugin:", filename);

const pluginId = Deno.openPlugin(filename);

// This passes
assert((Deno as any).core.ops()['notify_send'], "Notify ops do not exist while they should be available.");

Deno.close(pluginId);

// This fails
assert(!(Deno as any).core.ops()['notify_send'], "Notify ops still exist while they should be removed.");

Note: I ran this with deno 1.0.3, v8 8.4.300, typescript 3.9.2

[bartlomieju]

CC @afinch7

[afinch7]

This is less of a bug and more of a oversight. I'm working on a fix.

[afinch7]

We don't currently have any way to unregister ops in core. We need to address that first or not allow closing plugins. I struggle to find any good reason to need to call close on plugin.

[Pandawan]

I wanted to close the plugin so I could test different approaches to plugin loading: testing my plugin when loaded or not, when loaded manually using Deno.openPlugin(), or when auto-loaded with deno-plugin-prepare, etc.

But since Deno.test() instances "share" the same plugins (even across files), I needed a way to register/unregister the plugin at the start/end of tests.

I'm sure there are other use cases, but I think in general it's good to have a way to be able to reset your state/resource usage, including plugins.

[ry]

It's unclear to me whether it's useful to unload plugins. It's definitely going to complicate matters...

[Pandawan]

How so? I don't see much harm in allowing runtime removal of plugins, especially if you can already do it on the rust side (as was added in #6214). Unless there's some hidden cost/complication that I'm not aware of (which is likely).

[ry]

For example, if the plugin adds a resource to the resource table, which has a Drop implementation in the plugin DLL. We need to make sure the resource is destroyed before the DLL is unloaded.

[afinch7]

After looking at the complexity involved in unloading plugins, it might make more sense to just support loading the same plugin more than once via namespacing.