Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
19 changes: 16 additions & 3 deletions src/bun.js/bindings/webcore/JSWorker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,28 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor::

auto envValue = optionsObject->getIfPropertyExists(lexicalGlobalObject, Identifier::fromString(vm, "env"_s));
RETURN_IF_EXCEPTION(throwScope, {});
// for now, we don't permit SHARE_ENV, because the behavior isn't implemented
if (envValue && !(envValue.isObject() || envValue.isUndefinedOrNull())) {

// Check if envValue is the SHARE_ENV symbol (nodejs.worker_threads.SHARE_ENV)
bool isShareEnv = false;
if (envValue && envValue.isSymbol()) {
auto* symbol = asSymbol(envValue);
// Check if this is the global symbol for nodejs.worker_threads.SHARE_ENV
auto description = symbol->description();
if (!description.isEmpty() && description == "nodejs.worker_threads.SHARE_ENV"_s) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not correct. It needs to use symbolRegistry()->keyFor or something along those lines. It needs to use the sybmol registry and check the two Symbol's are equal and not the description value

isShareEnv = true;
}
}

if (envValue && !(envValue.isObject() || envValue.isUndefinedOrNull() || isShareEnv)) {
return Bun::ERR::INVALID_ARG_TYPE(throwScope, globalObject, "options.env"_s, "object or one of undefined, null, or worker_threads.SHARE_ENV"_s, envValue);
}
JSObject* envObject = nullptr;

if (envValue && envValue.isCell()) {
if (envValue && envValue.isCell() && !isShareEnv) {
envObject = jsDynamicCast<JSC::JSObject*>(envValue);
} else if (globalObject->m_processEnvObject.isInitialized()) {
// For both SHARE_ENV and default cases, use the process environment
// SHARE_ENV means share the live parent environment
envObject = globalObject->processEnvObject();
}

Expand Down
2 changes: 1 addition & 1 deletion src/js/node/worker_threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const {
// node:worker_threads instance instead of the Web Worker instance.
Worker: new (...args: [...ConstructorParameters<typeof globalThis.Worker>, nodeWorker: Worker]) => WebWorker;
};
const SHARE_ENV = Symbol("nodejs.worker_threads.SHARE_ENV");
const SHARE_ENV = Symbol.for("nodejs.worker_threads.SHARE_ENV");

const isMainThread = Bun.isMainThread;
const {
Expand Down
147 changes: 147 additions & 0 deletions test/js/node/worker_threads/share-env.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
import { expect, test } from "bun:test";
import { SHARE_ENV, Worker } from "worker_threads";

test("SHARE_ENV symbol should be accepted as env option", async () => {
// This test verifies that the SHARE_ENV symbol is properly accepted
// as the env option in worker threads, fixing the issue where it was
// incorrectly rejected as an invalid type

const worker = new Worker(
`
const { parentPort } = require('worker_threads');
// Send back the current environment variable to verify SHARE_ENV works
parentPort.postMessage({
PATH: process.env.PATH ? 'present' : 'absent',
NODE_ENV: process.env.NODE_ENV
});
`,
{
eval: true,
env: SHARE_ENV,
},
);

const message = await new Promise((resolve, reject) => {
worker.on("message", resolve);
worker.on("error", reject);
setTimeout(() => reject(new Error("Test timeout")), 5000);
});

// Verify that environment variables are shared from parent process
expect(message).toHaveProperty("PATH");
expect((message as any).PATH).toBe("present");

await worker.terminate();
});

test("SHARE_ENV enables true environment sharing", async () => {
// Set a unique test variable in the parent
const testVar = `TEST_VAR_${Date.now()}`;
process.env[testVar] = "parent_value";

const worker = new Worker(
`
const { parentPort } = require('worker_threads');
// Worker should see the parent's environment variable
parentPort.postMessage({
testVar: process.env["${testVar}"],
hasTestVar: "${testVar}" in process.env
});
`,
{
eval: true,
env: SHARE_ENV,
},
);

const message = await new Promise((resolve, reject) => {
worker.on("message", resolve);
worker.on("error", reject);
setTimeout(() => reject(new Error("Test timeout")), 5000);
});

// Verify the worker can see the parent's environment variable
expect((message as any).hasTestVar).toBe(true);
expect((message as any).testVar).toBe("parent_value");

// Clean up
delete process.env[testVar];
await worker.terminate();
});

test("SHARE_ENV should be the correct symbol", () => {
// Verify that SHARE_ENV is the expected symbol
expect(typeof SHARE_ENV).toBe("symbol");
expect(SHARE_ENV.description).toBe("nodejs.worker_threads.SHARE_ENV");
});

test("non-SHARE_ENV symbols should still be rejected", async () => {
const someOtherSymbol = Symbol("other.symbol");

expect(() => {
new Worker("", {
eval: true,
env: someOtherSymbol as any,
});
}).toThrow(
/The "options\.env" property must be of type object or one of undefined, null, or worker_threads\.SHARE_ENV/,
);
});

test("other env option types should still work", async () => {
// Test that regular object env still works
const worker1 = new Worker(
`
const { parentPort } = require('worker_threads');
parentPort.postMessage(process.env.CUSTOM_VAR);
`,
{
eval: true,
env: { CUSTOM_VAR: "custom_value" },
},
);

const message1 = await new Promise((resolve, reject) => {
worker1.on("message", resolve);
worker1.on("error", reject);
setTimeout(() => reject(new Error("Test timeout")), 5000);
});

expect(message1).toBe("custom_value");
await worker1.terminate();

// Test that undefined env still works
const worker2 = new Worker(
`
const { parentPort } = require('worker_threads');
parentPort.postMessage('success');
`,
{
eval: true,
env: undefined,
},
);

const message2 = await new Promise((resolve, reject) => {
worker2.on("message", resolve);
worker2.on("error", reject);
setTimeout(() => reject(new Error("Test timeout")), 5000);
});

expect(message2).toBe("success");
await worker2.terminate();

// Test that null env still works
expect(() => {
new Worker(
`
const { parentPort } = require('worker_threads');
parentPort.postMessage('success');
`,
{
eval: true,
env: null,
},
);
}).not.toThrow();
});