refactor(ext/node): worker_threads.isMainThread setup (#22785)

This commit changes how we figure out if we're running on main
thread in `node:worker_threads` module. Instead of relying on quirky
"magic variable" for a name to check if we're on main thread, we are
now explicitly passing this information during bootstrapping of the
runtime. As a side effect, `WorkerOptions.name` is more useful
and matches what Node.js does more closely (though not fully).

Towards https://github.com/denoland/deno/issues/22783
This commit is contained in:
Bartek Iwańczuk 2024-03-08 16:31:11 +00:00 committed by GitHub
parent d5b01e4158
commit 93b9d114a4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 22 additions and 16 deletions

View file

@ -13,6 +13,7 @@ let initialized = false;
function initialize(
usesLocalNodeModulesDir,
argv0,
runningOnMainThread,
) {
if (initialized) {
throw Error("Node runtime already initialized");
@ -37,7 +38,7 @@ function initialize(
// FIXME(bartlomieju): not nice to depend on `Deno` namespace here
// but it's the only way to get `args` and `version` and this point.
internals.__bootstrapNodeProcess(argv0, Deno.args, Deno.version);
internals.__initWorkerThreads();
internals.__initWorkerThreads(runningOnMainThread);
internals.__setupChildProcessIpcChannel();
// `Deno[Deno.internal].requireImpl` will be unreachable after this line.
delete internals.requireImpl;

View file

@ -33,6 +33,7 @@ const {
StringPrototypeMatch,
StringPrototypeReplaceAll,
StringPrototypeToString,
StringPrototypeTrim,
SafeWeakMap,
SafeRegExp,
SafeMap,
@ -55,7 +56,6 @@ export interface WorkerOptions {
codeRangeSizeMb?: number;
stackSizeMb?: number;
};
// deno-lint-ignore prefer-primordials
eval?: boolean;
transferList?: Transferable[];
@ -133,11 +133,9 @@ function toFileUrl(path: string): URL {
let threads = 0;
const privateWorkerRef = Symbol("privateWorkerRef");
const PRIVATE_WORKER_THREAD_NAME = "$DENO_STD_NODE_WORKER_THREAD";
class NodeWorker extends EventEmitter {
#id = 0;
// TODO(satyarohith): remove after https://github.com/denoland/deno/pull/22785 lands
#name = PRIVATE_WORKER_THREAD_NAME;
#name = "";
#refCount = 1;
#messagePromise = undefined;
#controlPromise = undefined;
@ -187,6 +185,13 @@ class NodeWorker extends EventEmitter {
}
}
// TODO(bartlomieu): this doesn't match the Node.js behavior, it should be
// `[worker {threadId}] {name}` or empty string.
let name = StringPrototypeTrim(options?.name ?? "");
if (options?.eval) {
name = "[worker eval]";
}
this.#name = name;
const id = op_create_worker(
{
// deno-lint-ignore prefer-primordials
@ -378,10 +383,8 @@ type ParentPort = typeof self & NodeEventTarget;
// deno-lint-ignore no-explicit-any
let parentPort: ParentPort = null as any;
internals.__initWorkerThreads = () => {
isMainThread =
// deno-lint-ignore no-explicit-any
(globalThis as any).name !== PRIVATE_WORKER_THREAD_NAME;
internals.__initWorkerThreads = (runningOnMainThread: boolean) => {
isMainThread = runningOnMainThread;
defaultExport.isMainThread = isMainThread;
// fake resourceLimits
@ -394,8 +397,6 @@ internals.__initWorkerThreads = () => {
defaultExport.resourceLimits = resourceLimits;
if (!isMainThread) {
// deno-lint-ignore no-explicit-any
delete (globalThis as any).name;
const listeners = new SafeWeakMap<
// deno-lint-ignore no-explicit-any
(...args: any[]) => void,
@ -411,12 +412,16 @@ internals.__initWorkerThreads = () => {
"message",
),
(result) => {
// TODO(bartlomieju): just so we don't error out here. It's still racy,
// but should be addressed by https://github.com/denoland/deno/issues/22783
// shortly.
const data = result[0].data ?? {};
// TODO(kt3k): The below values are set asynchronously
// using the first message from the parent.
// This should be done synchronously.
threadId = result[0].data.threadId;
workerData = result[0].data.workerData;
environmentData = result[0].data.environmentData;
threadId = data.threadId;
workerData = data.workerData;
environmentData = data.environmentData;
defaultExport.threadId = threadId;
defaultExport.workerData = workerData;

View file

@ -774,7 +774,7 @@ function bootstrapMainRuntime(runtimeOptions) {
ObjectDefineProperty(globalThis, "Deno", core.propReadOnly(finalDenoNs));
if (nodeBootstrap) {
nodeBootstrap(hasNodeModulesDir, argv0);
nodeBootstrap(hasNodeModulesDir, argv0, /* runningOnMainThread */ true);
}
if (future) {
@ -909,7 +909,7 @@ function bootstrapWorkerRuntime(
ObjectDefineProperty(globalThis, "Deno", core.propReadOnly(finalDenoNs));
if (nodeBootstrap) {
nodeBootstrap(hasNodeModulesDir, argv0);
nodeBootstrap(hasNodeModulesDir, argv0, /* runningOnMainThread */ false);
}
}