-
Notifications
You must be signed in to change notification settings - Fork 12
feat: expose nodeWorkerArgs config #108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,17 +9,12 @@ import { | |
SET_HOST_CONTEXT, | ||
STORE, | ||
} from './call-context.ts'; | ||
import { | ||
type InternalConfig, | ||
PluginOutput, | ||
SAB_BASE_OFFSET, | ||
SharedArrayBufferSection, | ||
} from './interfaces.ts'; | ||
import { type InternalConfig, PluginOutput, SAB_BASE_OFFSET, SharedArrayBufferSection } from './interfaces.ts'; | ||
import { WORKER_URL } from './worker-url.ts'; | ||
import { Worker } from 'node:worker_threads'; | ||
import { CAPABILITIES } from './polyfills/deno-capabilities.ts'; | ||
import { EXTISM_ENV } from './foreground-plugin.ts'; | ||
import { HttpContext } from './http-context.ts' | ||
import { HttpContext } from './http-context.ts'; | ||
|
||
// Firefox has not yet implemented Atomics.waitAsync, but we can polyfill | ||
// it using a worker as a one-off. | ||
|
@@ -37,7 +32,7 @@ const AtomicsWaitAsync = | |
|
||
const blob = new (Blob as any)([src], { type: 'text/javascript' }); | ||
const url = URL.createObjectURL(blob); | ||
const w = new Worker(url); | ||
const w = new Worker(url, { execArgv: [] }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This worker is a fallback for |
||
return (ia: any, index, value) => { | ||
const promise = new Promise((resolve) => { | ||
w.once('message', (data) => { | ||
|
@@ -85,7 +80,7 @@ class BackgroundPlugin { | |
async #handleTimeout() { | ||
// block new requests from coming in & the current request from settling | ||
const request = this.#request; | ||
this.#request = [() => { }, () => { }]; | ||
this.#request = [() => {}, () => {}]; | ||
|
||
const timedOut = {}; | ||
const failed = {}; | ||
|
@@ -208,7 +203,7 @@ class BackgroundPlugin { | |
await this.#handleTimeout(); | ||
} | ||
}, | ||
() => { }, | ||
() => {}, | ||
); | ||
|
||
this.worker.postMessage({ | ||
|
@@ -298,7 +293,7 @@ class BackgroundPlugin { | |
// | ||
// - https://github.com/nodejs/node/pull/44409 | ||
// - https://github.com/denoland/deno/issues/14786 | ||
const timer = setInterval(() => { }, 0); | ||
const timer = setInterval(() => {}, 0); | ||
try { | ||
if (!func) { | ||
throw Error(`Plugin error: host function "${ev.namespace}" "${ev.func}" does not exist`); | ||
|
@@ -423,7 +418,7 @@ class RingBufferWriter { | |
|
||
signal() { | ||
const old = Atomics.load(this.flag, 0); | ||
while (Atomics.compareExchange(this.flag, 0, old, this.outputOffset) === old) { } | ||
while (Atomics.compareExchange(this.flag, 0, old, this.outputOffset) === old) {} | ||
Atomics.notify(this.flag, 0, 1); | ||
} | ||
|
||
|
@@ -546,9 +541,9 @@ async function createWorker( | |
names: string[], | ||
modules: WebAssembly.Module[], | ||
sharedData: SharedArrayBuffer, | ||
onworker: (_w: Worker) => void = (_w: Worker) => { }, | ||
onworker: (_w: Worker) => void = (_w: Worker) => {}, | ||
): Promise<Worker> { | ||
const worker = new Worker(WORKER_URL); | ||
const worker = new Worker(WORKER_URL, opts.nodeWorkerArgs); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the actual worker. We've normalized |
||
onworker(worker); | ||
|
||
await new Promise((resolve, reject) => { | ||
|
@@ -595,7 +590,7 @@ function timeout(ms: number | null, sentinel: any) { | |
|
||
async function terminateWorker(w: Worker) { | ||
if (typeof (globalThis as any).Bun !== 'undefined') { | ||
const timer = setTimeout(() => { }, 10); | ||
const timer = setTimeout(() => {}, 10); | ||
await w.terminate(); | ||
clearTimeout(timer); | ||
} else { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -124,6 +124,23 @@ export interface Plugin { | |
reset(): Promise<boolean>; | ||
} | ||
|
||
/** | ||
* Arguments to be passed to `node:worker_threads.Worker` when `runInWorker: true`. | ||
*/ | ||
export interface NodeWorkerArgs { | ||
name?: string; | ||
execArgv?: string[]; | ||
argv?: string[]; | ||
env?: Record<string, string>; | ||
resourceLimits?: { | ||
maxOldGenerationSizeMb?: number; | ||
maxYoungGenerationSizeMb?: number; | ||
codeRangeSizeMb?: number; | ||
stackSizeMb?: number; | ||
}; | ||
[k: string]: any; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is pretty much copied from Node's API, but neither |
||
} | ||
|
||
/** | ||
* Options for initializing an Extism plugin. | ||
*/ | ||
|
@@ -214,6 +231,16 @@ export interface ExtismPluginOptions { | |
* headers for HTTP requests made using `extism:host/env::http_request` | ||
*/ | ||
allowHttpResponseHeaders?: boolean; | ||
|
||
/** | ||
* Arguments to pass to the `node:worker_threads.Worker` instance when `runInWorker: true`. | ||
* | ||
* This is particularly useful for changing `process.execArgv`, which controls certain startup | ||
* behaviors in node (`--import`, `--require`, warnings.) | ||
* | ||
* If not set, defaults to removing the current `execArgv` and disabling node warnings. | ||
*/ | ||
nodeWorkerArgs?: NodeWorkerArgs; | ||
} | ||
|
||
export type MemoryOptions = { | ||
|
@@ -259,6 +286,7 @@ export interface InternalConfig extends Required<NativeManifestOptions> { | |
wasiEnabled: boolean; | ||
sharedArrayBufferSize: number; | ||
allowHttpResponseHeaders: boolean; | ||
nodeWorkerArgs: NodeWorkerArgs; | ||
} | ||
|
||
export interface InternalWasi { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,6 +99,13 @@ export async function createPlugin( | |
opts.config = opts.config || manifestOpts.config || {}; | ||
opts.memory = opts.memory || manifestOpts.memory || {}; | ||
opts.timeoutMs = opts.timeoutMs || manifestOpts.timeoutMs || null; | ||
opts.nodeWorkerArgs = Object.assign( | ||
{ | ||
name: 'extism plugin', | ||
execArgv: ['--disable-warning=ExperimentalWarning'], | ||
}, | ||
opts.nodeWorkerArgs || {}, | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🍎 We normalize |
||
|
||
if (opts.allowedHosts.length && !opts.runInWorker) { | ||
if (!(WebAssembly as any).Suspending) { | ||
|
@@ -142,6 +149,7 @@ export async function createPlugin( | |
timeoutMs: opts.timeoutMs, | ||
memory: opts.memory, | ||
allowHttpResponseHeaders: !!opts.allowHttpResponseHeaders, | ||
nodeWorkerArgs: opts.nodeWorkerArgs || {}, | ||
}; | ||
|
||
return (opts.runInWorker ? _createBackgroundPlugin : _createForegroundPlugin)(ic, names, moduleData); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just format
caught a bunch of missing semicolons, apologies. I'll call out the big changes.