Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 9 additions & 4 deletions src/foreground-plugin.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { CallContext, RESET, GET_BLOCK, BEGIN, END, ENV, STORE } from './call-context.ts';
import { PluginOutput, type InternalConfig } from './interfaces.ts';
import { PluginOutput, type InternalConfig, InternalWasi } from './interfaces.ts';
import { loadWasi } from './polyfills/deno-wasi.ts';

export const EXTISM_ENV = 'extism:host/env';
Expand All @@ -11,11 +11,13 @@ export class ForegroundPlugin {
#modules: InstantiatedModule[];
#names: string[];
#active: boolean = false;
#wasi: InternalWasi | null;

constructor(context: CallContext, names: string[], modules: InstantiatedModule[]) {
constructor(context: CallContext, names: string[], modules: InstantiatedModule[], wasi: InternalWasi | null) {
this.#context = context;
this.#names = names;
this.#modules = modules;
this.#wasi = wasi;
}

async reset(): Promise<boolean> {
Expand Down Expand Up @@ -144,7 +146,10 @@ export class ForegroundPlugin {
}

async close(): Promise<void> {
// noop
if (this.#wasi) {
await this.#wasi.close();
this.#wasi = null;
}
}
}

Expand Down Expand Up @@ -191,5 +196,5 @@ export async function createForegroundPlugin(
}),
);

return new ForegroundPlugin(context, names, instances);
return new ForegroundPlugin(context, names, instances, wasi);
}
7 changes: 6 additions & 1 deletion src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,11 @@ export interface ExtismPluginOptions {
/**
* Whether or not to run the Wasm module in a Worker thread. Requires
* {@link Capabilities#hasWorkerCapability | `CAPABILITIES.hasWorkerCapability`} to
* be true.
* be true. Defaults to false.
*
* This feature is marked experimental as we work out [a bug](https://github.com/extism/js-sdk/issues/46).
*
* @experimental
*/
runInWorker?: boolean;

Expand Down Expand Up @@ -190,6 +194,7 @@ export interface InternalConfig {
export interface InternalWasi {
importObject(): Promise<Record<string, WebAssembly.ImportValue>>;
initialize(instance: WebAssembly.Instance): Promise<void>;
close(): Promise<void>;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ export async function createPlugin(
opts.config ??= {};
opts.fetch ??= fetch;

opts.runInWorker ??= CAPABILITIES.hasWorkerCapability;
// TODO(chrisdickinson): reset this to `CAPABILITIES.hasWorkerCapability` once we've fixed https://github.com/extism/js-sdk/issues/46.
opts.runInWorker ??= false;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(It's a blink-and-you'll-miss-it diff – this is what actually changes the default threading behavior of the plugin.)

if (opts.runInWorker && !CAPABILITIES.hasWorkerCapability) {
throw new Error(
'Cannot enable off-thread wasm; current context is not `crossOriginIsolated` (see https://mdn.io/crossOriginIsolated)',
Expand Down
4 changes: 4 additions & 0 deletions src/polyfills/browser-wasi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ export async function loadWasi(
return context.wasiImport;
},

async close() {
// noop
},

async initialize(instance: WebAssembly.Instance) {
const memory = instance.exports.memory as WebAssembly.Memory;

Expand Down
21 changes: 17 additions & 4 deletions src/polyfills/deno-wasi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,34 @@ import { closeSync } from 'node:fs';

async function createDevNullFDs() {
const [stdin, stdout] = await Promise.all([open(devNull, 'r'), open(devNull, 'w')]);

let needsClose = true;
const fr = new globalThis.FinalizationRegistry((held: number) => {
try {
closeSync(held);
if (needsClose) closeSync(held);
} catch {
// The fd may already be closed.
}
});
fr.register(stdin, stdin.fd);
fr.register(stdout, stdout.fd);

return [stdin.fd, stdout.fd, stdout.fd];
return {
async close() {
needsClose = false;
await Promise.all([stdin.close(), stdout.close()]).catch(() => {});
},
fds: [stdin.fd, stdout.fd, stdout.fd],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deno, unlike Node, runs its "leaked file descriptor" check before the FinalizationRegistry has a chance to fire; so we add the explicit close here.

};
}

export async function loadWasi(
allowedPaths: { [from: string]: string },
enableWasiOutput: boolean,
): Promise<InternalWasi> {
const [stdin, stdout, stderr] = enableWasiOutput ? [0, 1, 2] : await createDevNullFDs();
const {
close,
fds: [stdin, stdout, stderr],
} = enableWasiOutput ? { async close() {}, fds: [0, 1, 2] } : await createDevNullFDs();
const context = new Context({
preopens: allowedPaths,
exitOnReturn: false,
Expand All @@ -38,6 +47,10 @@ export async function loadWasi(
return context.exports;
},

async close() {
await close();
},

async initialize(instance: WebAssembly.Instance) {
const memory = instance.exports.memory as WebAssembly.Memory;

Expand Down
45 changes: 34 additions & 11 deletions src/polyfills/node-wasi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,44 @@ import { closeSync } from 'node:fs';

async function createDevNullFDs() {
const [stdin, stdout] = await Promise.all([open(devNull, 'r'), open(devNull, 'w')]);
let needsClose = true;
// TODO: make this check always run when bun fixes [1], so `fs.promises.open()` returns a `FileHandle` as expected.
// [1]: https://github.com/oven-sh/bun/issues/5918
let close = async () => {
closeSync(stdin as any);
closeSync(stdout as any);
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... And Bun returns the wrong type from fs.promises.open, so we guard against that here.

if (typeof stdin !== 'number') {
const fr = new globalThis.FinalizationRegistry((held: number) => {
try {
if (needsClose) closeSync(held);
} catch {
// The fd may already be closed.
}
});

const fr = new globalThis.FinalizationRegistry((held: number) => {
try {
closeSync(held);
} catch {
// The fd may already be closed.
}
});
fr.register(stdin, stdin.fd);
fr.register(stdout, stdout.fd);
fr.register(stdin, stdin.fd);
fr.register(stdout, stdout.fd);
close = async () => {
needsClose = false;
await Promise.all([stdin.close(), stdout.close()]).catch(() => {});
};
}

return [stdin.fd, stdout.fd, stdout.fd];
return {
close,
fds: [stdin.fd, stdout.fd, stdout.fd],
};
}

export async function loadWasi(
allowedPaths: { [from: string]: string },
enableWasiOutput: boolean,
): Promise<InternalWasi> {
const [stdin, stdout, stderr] = enableWasiOutput ? [0, 1, 2] : await createDevNullFDs();
const {
close,
fds: [stdin, stdout, stderr],
} = enableWasiOutput ? { async close() {}, fds: [0, 1, 2] } : await createDevNullFDs();

const context = new WASI({
version: 'preview1',
Expand All @@ -39,6 +58,10 @@ export async function loadWasi(
return context.wasiImport;
},

async close() {
await close();
},

async initialize(instance: WebAssembly.Instance) {
const memory = instance.exports.memory as WebAssembly.Memory;

Expand Down