Skip to content

Commit

Permalink
fix: mark threads as experimental (#49)
Browse files Browse the repository at this point in the history
Because we're still working out kinks in the implementation [1], users can
specifically request that plugins run in the background, but they no longer do
so by default where available. Once [1] is fixed, we're likely to make threading
default again.

[1]: #46
  • Loading branch information
chrisdickinson authored Jan 8, 2024
1 parent 25e17a4 commit b38021d
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 21 deletions.
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;
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],
};
}

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);
};
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

0 comments on commit b38021d

Please sign in to comment.