Skip to content
23 changes: 20 additions & 3 deletions src/dotenv/env_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -595,14 +595,31 @@ impl Loader {
}
}

/// A `Worker`'s loader: the map, with the explicit entries marked loaded so a pipe is not read twice.
/// A `Worker`'s copy: the map plus what was already loaded, so the worker thread
/// does not read `environ`, `.env` files or an `--env-file` pipe a second time.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub fn clone_for_worker(&self) -> Result<Loader, AllocError> {
Ok(Loader {
map: self.map.clone_with_allocator()?,
default_files_loaded: EnumSet::empty(),
default_files_loaded: self.default_files_loaded,
custom_files_loaded: self.custom_files_loaded.clone()?,
quiet: false,
did_load_process: false,
did_load_process: self.did_load_process,
reject_unauthorized: Cell::new(None),
aws_credentials: None,
})
}

/// An empty loader for a `Worker` given `{ env }`: the caller fills the map, and
/// every env source counts as loaded so nothing overlays it later.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub fn for_worker_env(&self, capacity: usize) -> Result<Loader, AllocError> {
let mut map = Map::init();
map.ensure_unused_capacity(capacity)?;
Ok(Loader {
map,
default_files_loaded: EnumSet::all(),
custom_files_loaded: self.custom_files_loaded.clone()?,
quiet: false,
did_load_process: true,
reject_unauthorized: Cell::new(None),
aws_credentials: None,
})
Expand Down
30 changes: 14 additions & 16 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3687,22 +3687,20 @@ impl VirtualMachine {
self.transpiler_store.enabled = false;
}

if let Some(idx) = map.map.get_index(b"NODE_CHANNEL_FD") {
let (_, kv) = map.map.swap_remove_at(idx);
let fd_s = kv.value;
let advanced = map
.map
.get_index(b"NODE_CHANNEL_SERIALIZATION_MODE")
.map(|i| map.map.swap_remove_at(i).1)
.is_some_and(|v| &v.value[..] == b"advanced");
// Accept only
// non-negative values that fit in i31 (i.e. `0..=i32::MAX`).
// Parsing as `u32` then `as i32` would silently wrap values in
// `2^31..2^32` to a negative fd instead of taking the warn branch.
// The channel belongs to the process (its main thread). A worker
// sees the same inherited variables but must not open a second
// endpoint over the same fd (Node: no process.send() in workers).
if self.is_main_thread() {
// Only the main thread adopts the process's IPC channel and drops the
// variables (as Node does). A worker keeps what its env gave it;
// node:worker_threads reads `process.env.NODE_CHANNEL_FD` to stub `process.send`.
Comment thread
robobun marked this conversation as resolved.
Resolved
if self.is_main_thread() {
if let Some(idx) = map.map.get_index(b"NODE_CHANNEL_FD") {
let (_, kv) = map.map.swap_remove_at(idx);
let fd_s = kv.value;
let advanced = map
.map
.get_index(b"NODE_CHANNEL_SERIALIZATION_MODE")
.map(|i| map.map.swap_remove_at(i).1)
.is_some_and(|v| &v.value[..] == b"advanced");
// Non-negative i31 only: `u32` then `as i32` would wrap `2^31..` to a
// negative fd instead of warning.
Comment thread
robobun marked this conversation as resolved.
Resolved
match bun_core::fmt::parse_int::<i32>(&fd_s, 10)
.ok()
.filter(|&n| n >= 0)
Expand Down
4 changes: 4 additions & 0 deletions src/jsc/bindings/JSEnvironmentVariableMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,10 @@ JSC_DEFINE_HOST_FUNCTION(jsEditWindowsEnvVar, (JSGlobalObject * global, JSC::Cal
auto scope = DECLARE_THROW_SCOPE(global->vm());
ASSERT(callFrame->argumentCount() == 2);
ASSERT(callFrame->uncheckedArgument(0).isString());
// Only the main thread writes through to the OS environment (Node: a worker env is a MapKVStore).
auto* context = defaultGlobalObject(global)->scriptExecutionContext();
if (context && !context->isMainThread())
return JSValue::encode(jsUndefined());
WTF::String string1 = callFrame->uncheckedArgument(0).toWTFString(global);
RETURN_IF_EXCEPTION(scope, {});
JSValue arg2 = callFrame->uncheckedArgument(1);
Expand Down
43 changes: 5 additions & 38 deletions src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -571,44 +571,11 @@ extern "C" JSC::JSGlobalObject* Zig__GlobalObject__create(void* console_client,
const auto initializeWorker = [&](WebCore::WorkerMessagingProxy& worker) -> void {
auto& options = worker.options();

if (options.env.has_value()) {
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
HashMap<String, String> map = *std::exchange(options.env, std::nullopt);
auto size = map.size();

// In theory, a GC could happen before we finish putting all the properties on the object.
// So we use a MarkedArgumentBuffer to ensure that the strings are not collected and we immediately put them on the object.
MarkedArgumentBuffer strings;
strings.ensureCapacity(size);
for (const auto& value : map.values()) {
strings.append(jsString(vm, value));
}

#if OS(WINDOWS)
JSC::JSObject* env = size
? JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), size >= JSFinalObject::maxInlineCapacity ? JSFinalObject::maxInlineCapacity : size)
: JSC::constructEmptyObject(globalObject);
#else
// Same exotic object as the main thread so writes inside the
// worker coerce to string, reject symbol keys, and validate
// defineProperty like Node's EnvSetter/EnvDefiner.
auto* envStructure = Bun::JSEnvironmentVariableMap::createStructure(vm, globalObject, globalObject->objectPrototype());
JSC::JSObject* env = Bun::JSEnvironmentVariableMap::create(vm, envStructure);
#endif
size_t i = 0;
for (auto k : map) {
// Numeric env keys hit putDirectIndex → defineOwnProperty (declares a
// ThrowScope). Seeded values are JSStrings, so this throws only on OOM
// or under a termination already requested for this starting worker.
env->putDirectMayBeIndex(globalObject, JSC::Identifier::fromString(vm, WTF::move(k.key)), strings.at(i++));
if (scope.exception()) [[unlikely]]
break;
}
globalObject->m_processEnvObject.set(vm, globalObject, env);
} else if (options.sharedEnvStore) {
// worker_threads SHARE_ENV: join the env tree the spawning thread
// resolved. Consumed like options.env, and published on the context
// before the view, which resolves its store through the context.
// options.env already seeded this VM's env loader (WebWorker__create); process.env
// is built lazily from it by createEnvironmentVariablesMap, as on the main thread.
Comment thread
robobun marked this conversation as resolved.
Resolved
if (options.sharedEnvStore) {
Comment thread
robobun marked this conversation as resolved.
Resolved
// worker_threads SHARE_ENV: join the env tree the spawning thread resolved.
// Published on the context before the view, which resolves its store through it.
Comment thread
robobun marked this conversation as resolved.
Resolved
RefPtr<Bun::SharedEnvStore> store = std::exchange(options.sharedEnvStore, nullptr);
globalObject->scriptExecutionContext()->setSharedEnvStore(*store);
globalObject->m_processEnvObject.set(vm, globalObject, Bun::createSharedEnvironmentVariablesMap(globalObject).getObject());
Expand Down
7 changes: 4 additions & 3 deletions src/jsc/bindings/webcore/JSWorker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,14 +285,15 @@ template<> __attribute__((minsize)) JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES
envObject->methodTable()->getOwnPropertyNames(envObject, lexicalGlobalObject, keys, JSC::DontEnumPropertiesMode::Exclude);
RETURN_IF_EXCEPTION(throwScope, {});

HashMap<String, String> env;
Vector<std::pair<String, String>> env;
env.reserveInitialCapacity(keys.size());

for (const auto& key : keys) {
JSValue value = envObject->get(lexicalGlobalObject, key);
RETURN_IF_EXCEPTION(throwScope, {});
String str = value.toWTFString(lexicalGlobalObject).isolatedCopy();
String str = value.toWTFString(lexicalGlobalObject);
RETURN_IF_EXCEPTION(throwScope, {});
env.add(key.impl()->isolatedCopy(), str);
env.append({ key.string(), WTF::move(str) });
}

options.env.emplace(WTF::move(env));
Expand Down
22 changes: 20 additions & 2 deletions src/jsc/bindings/webcore/WorkerMessagingProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ void* WebWorker__create(
StringImpl** execArgvPtr,
size_t execArgvLen,
BunString* preloadModulesPtr,
size_t preloadModulesLen);
size_t preloadModulesLen,
bool hasEnv,
const BunString* envPairsPtr,
size_t envPairsLen);
// Raise a TerminationException in the worker VM at its next safepoint and wake its loop. Any thread.
void WebWorker__requestTermination(void*);
// Toggle the keep-alive this worker holds on the parent event loop. Parent thread.
Expand Down Expand Up @@ -140,6 +143,17 @@ ExceptionOr<void> WorkerMessagingProxy::startWorkerGlobalScope(const String& scr
})
.value_or(std::span<WTF::StringImpl*> {});

// [key, value, ...]; WebWorker__create copies the bytes, so these only live across the call.
Vector<BunString> envPairs;
bool hasEnv = m_options.env.has_value();
if (hasEnv) {
envPairs.reserveInitialCapacity(m_options.env->size() * 2);
for (auto& [key, value] : *m_options.env) {
envPairs.append(Bun::toString(key));
envPairs.append(Bun::toString(value));
}
}

// The thread holds a ref on the proxy until releaseWorkerThread().
ref();
BunString errorMessage = BunStringEmpty;
Expand All @@ -163,8 +177,12 @@ ExceptionOr<void> WorkerMessagingProxy::startWorkerGlobalScope(const String& scr
execArgv.data(),
execArgv.size(),
preloadModules.begin(),
preloadModules.size());
preloadModules.size(),
hasEnv,
envPairs.begin(),
envPairs.size() / 2);
m_options.preloadModules.clear();
m_options.env = std::nullopt;

if (!m_workerThread) {
m_state.store(State::Closed);
Expand Down
4 changes: 3 additions & 1 deletion src/jsc/bindings/webcore/WorkerOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ struct WorkerOptions {
// Objects transferred for either data or environmentData in the transferList
Vector<TransferredMessagePort> dataMessagePorts;
Vector<String> preloadModules;
std::optional<HashMap<String, String>> env;
// The worker's whole initial environment in enumeration order, consumed on the parent
// thread by WebWorker__create. If nullopt, the worker clones the parent's env loader.
Comment thread
robobun marked this conversation as resolved.
Resolved
std::optional<Vector<std::pair<String, String>>> env;
Vector<String> argv;
// If nullopt, inherit execArgv from the parent thread
std::optional<Vector<String>> execArgv;
Expand Down
52 changes: 44 additions & 8 deletions src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,24 @@
proxy_env_slots: jsc::rare_data::ProxyEnvSlots,
}

/// The loader for a worker given its environment as `[key, value, ...]` pairs,
/// in `process.env` enumeration order.
Comment thread
robobun marked this conversation as resolved.
Resolved
fn worker_env_loader(
parent: &bun_dotenv::Loader,
pairs: &[BunString],
) -> Result<bun_dotenv::Loader, bun_alloc::AllocError> {
let mut loader = parent.for_worker_env(pairs.len() / 2)?;
Comment thread
robobun marked this conversation as resolved.
Resolved
for pair in pairs.chunks_exact(2) {

Check failure on line 142 in src/jsc/web_worker.rs

View workflow job for this annotation

GitHub Actions / cargo clippy

using `chunks_exact` with a constant chunk size
let key = pair[0].to_utf8();
if key.is_empty() {
continue;
}
let value = pair[1].to_utf8();
loader.map.put(&key, &value)?;
}
Ok(loader)
}

enum EntryOutcome {
Continue,
/// The entry module rejected and no handler took it: the worker exits.
Expand Down Expand Up @@ -279,6 +297,9 @@
/// keep-alive on the parent event loop, register as a child of the parent VM,
/// and spawn the thread. On any failure returns null with `error_message`
/// set and nothing to clean up.
///
/// With `has_env`, the `env_pairs_len` interleaved key/value pairs at `env_ptr`
/// are the worker's whole environment; otherwise the parent's loader is cloned.
Comment thread
robobun marked this conversation as resolved.
Resolved
#[unsafe(export_name = "WebWorker__create")]
pub(crate) unsafe extern "C" fn create(
proxy: *mut c_void,
Expand All @@ -299,6 +320,9 @@
exec_argv_len: usize,
preload_modules_ptr: *const BunString,
preload_modules_len: usize,
has_env: bool,
env_ptr: *const BunString,
env_pairs_len: usize,
) -> *mut WebWorker {
jsc::mark_binding();
log!("[{}] create", this_context_id);
Expand Down Expand Up @@ -370,23 +394,35 @@
transform_options.allow_ffi_cc = Some(parent_allows_ffi_cc && flags.allow_ffi_cc);
}
}
// The worker's `process.env` starts as a copy of the parent's now (as in
// Node). Proxy-env values may be RefCountedEnvValue bytes owned by the
// parent's proxy_env_storage: snapshot slots + map under its lock so
// every slice copied is backed by a ref the snapshot holds.
// The worker's env loader (what its `process.env`, fetch() and Bun.spawn
// read): exactly the given environment, or a copy of the parent's now (as
// in Node), taken under the parent's proxy-env lock so every
// RefCountedEnvValue slice in the copy is backed by a ref the snapshot holds.
Comment thread
robobun marked this conversation as resolved.
Resolved
let mut proxy_env_slots = jsc::rare_data::ProxyEnvSlots::default();
let mut env_loader = {
let env_loader = if has_env {
// SAFETY: caller passed `2 * env_pairs_len` valid strings (or `(null,0)`);
// slice borrowed from C++ for the duration of this call.
let pairs: &[BunString] = unsafe { bun_core::ffi::slice(env_ptr, env_pairs_len * 2) };
match worker_env_loader(parent_ref.env_loader(), pairs) {
Ok(loader) => loader,
Err(_) => {
*error_message = BunString::static_("Out of memory");
return core::ptr::null_mut();
}
}
} else {
let parent_slots = parent_ref.proxy_env_storage.lock();
proxy_env_slots.clone_from(&parent_slots);
match parent_ref.env_loader().clone_for_worker() {
let mut loader = match parent_ref.env_loader().clone_for_worker() {
Ok(loader) => loader,
Err(_) => {
*error_message = BunString::static_("Out of memory");
return core::ptr::null_mut();
}
}
};
proxy_env_slots.sync_into(&mut loader.map);
loader
};
proxy_env_slots.sync_into(&mut env_loader.map);
let init = WorkerVmInit {
transform_options,
env_loader,
Expand Down
68 changes: 68 additions & 0 deletions test/js/web/workers/worker-env-proxy-fixture.mjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading