Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
7 changes: 6 additions & 1 deletion src/jsc/bindings/BunObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,12 @@ static JSValue constructWebViewObject(VM& vm, JSObject* bunObject);

static JSValue constructEnvObject(VM& vm, JSObject* object)
{
return uncheckedDowncast<Zig::GlobalObject>(object->globalObject())->processEnvObject();
// Same shape as constructEnv in BunProcess.cpp: the exception is left pending so the read throws and the property is retried next time.
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
JSObject* env = uncheckedDowncast<Zig::GlobalObject>(object->globalObject())->processEnvObject();
if (scope.exception()) [[unlikely]]
return {};
return env;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

JSC::EncodedJSValue flattenArrayOfBuffersIntoArrayBufferOrUint8Array(JSGlobalObject* lexicalGlobalObject, JSValue arrayValue, size_t maxLength, bool asUint8Array)
Expand Down
42 changes: 18 additions & 24 deletions src/jsc/bindings/BunProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1800,9 +1800,14 @@ static JSValue constructLoadEnvFile(VM& vm, JSObject* processObject)
return JSC::JSFunction::create(vm, globalObject, processObjectInternalsLoadEnvFileCodeGenerator(vm), globalObject);
}

// Lazy PropertyCallback builders that enter JS. reifyAllStaticProperties wraps these in
// DeferTerminationForAWhile; a non-termination throw is cleared+reported so the worker's
// reifyAllStaticProperties (node:worker_threads preload) doesn't leave a pending exception.
// Throw handling in the lazy process builders. Returning empty with the exception pending
// (constructEnv) reifies nothing, so the read throws and the next read retries; that builder's
// object is shared with Bun.env and import.meta.env and must not be cached in a failed state.
// The builders using the blocks below clear and report instead and reify undefined, so a failure
// inside the worker_threads preload's reifyAllStaticProperties (which defers termination around
// the builders) is reported rather than thrown out of an unrelated delete. Both use a
// TopExceptionScope: that bulk path does not check between builders, so a ThrowScope's simulated
// throw would fail the next builder under validateExceptionChecks.
Comment thread
robobun marked this conversation as resolved.
static JSValue callLazyProcessBuilder(VM& vm, JSC::JSGlobalObject* globalObject, JSC::FunctionExecutable* (*generator)(VM&), const JSC::ArgList& args)
{
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
Expand Down Expand Up @@ -2635,10 +2640,6 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb
return workers;
};

auto constructEnvironmentVariables = [&]() -> JSC::JSValue {
return globalObject->processEnvObject();
};

auto constructCpus = [&]() -> JSC::JSValue {
JSC::JSObject* cpus = JSC::constructEmptyArray(globalObject, nullptr);
RETURN_IF_EXCEPTION(scope, {});
Expand Down Expand Up @@ -2686,8 +2687,9 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb
RETURN_IF_EXCEPTION(scope, {});
report->putDirect(vm, JSC::Identifier::fromString(vm, "workers"_s), constructWorkers(), 0);
RETURN_IF_EXCEPTION(scope, {});
report->putDirect(vm, JSC::Identifier::fromString(vm, "environmentVariables"_s), constructEnvironmentVariables(), 0);
JSC::JSObject* environmentVariables = globalObject->processEnvObject();
RETURN_IF_EXCEPTION(scope, {});
report->putDirect(vm, JSC::Identifier::fromString(vm, "environmentVariables"_s), environmentVariables, 0);
report->putDirect(vm, JSC::Identifier::fromString(vm, "userLimits"_s), constructUserLimits(), 0);
RETURN_IF_EXCEPTION(scope, {});
report->putDirect(vm, JSC::Identifier::fromString(vm, "sharedObjects"_s), constructSharedObjects(), 0);
Expand Down Expand Up @@ -2772,8 +2774,7 @@ static JSValue constructProcessConfigObject(VM& vm, JSObject* processObject)
// v8_use_snapshot: 1
// }
// }
// Lazy property builder: exceptions must not propagate into
// reifyStaticProperty, which performs no exception check.
// Clears and reports rather than propagating; see callLazyProcessBuilder.
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
JSC::JSObject* config = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 2);
JSC::JSObject* variables = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 2);
Expand Down Expand Up @@ -3217,15 +3218,11 @@ static JSValue constructRevision(VM& vm, JSObject* processObject)
static JSValue constructEnv(VM& vm, JSObject* processObject)
{
auto* globalObject = uncheckedDowncast<Zig::GlobalObject>(processObject->globalObject());
// Lazy property builder: exceptions must not propagate into
// reifyStaticProperty, which performs no exception check.
// Propagates (returns empty with the exception pending); see callLazyProcessBuilder.
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
JSValue env = globalObject->processEnvObject();
if (auto* exception = scope.exception()) [[unlikely]] {
(void)scope.tryClearException();
Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception);
return JSC::jsUndefined();
}
JSObject* env = globalObject->processEnvObject();
if (scope.exception()) [[unlikely]]
return {};
return env;
}

Expand Down Expand Up @@ -4271,8 +4268,7 @@ JSC_DEFINE_HOST_FUNCTION(Process_stubFunctionReturningArray, (JSGlobalObject * g

static JSValue Process_stubEmptyArray(VM& vm, JSObject* processObject)
{
// Lazy property builder: exceptions must not propagate into
// reifyStaticProperty, which performs no exception check.
// Clears and reports rather than propagating; see callLazyProcessBuilder.
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
JSC::JSArray* array = JSC::constructEmptyArray(processObject->globalObject(), nullptr);
if (auto* exception = scope.exception()) [[unlikely]] {
Expand Down Expand Up @@ -4406,8 +4402,7 @@ extern "C" void Bun__Process__queueNextTick2(GlobalObject* globalObject, Encoded
// return require.cache.get(Bun.main)
static JSValue constructMainModuleProperty(VM& vm, JSObject* processObject)
{
// Lazy property builder: exceptions must not propagate into
// reifyStaticProperty, which performs no exception check.
// Clears and reports rather than propagating; see callLazyProcessBuilder.
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
auto* globalObject = defaultGlobalObject(processObject->globalObject());
auto* bun = globalObject->bunObject();
Expand Down Expand Up @@ -4446,8 +4441,7 @@ JSValue Process::constructNextTickFn(JSC::VM& vm, Zig::GlobalObject* globalObjec
args.append(JSC::JSFunction::create(vm, globalObject, 1, String(), jsFunctionDrainMicrotaskQueue, ImplementationVisibility::Private));
args.append(JSC::JSFunction::create(vm, globalObject, 1, String(), jsFunctionReportUncaughtException, ImplementationVisibility::Private));

// Lazy property builder: exceptions must not propagate into
// reifyStaticProperty, which performs no exception check.
// Clears and reports rather than propagating; see callLazyProcessBuilder.
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
JSValue nextTickFunction = JSC::profiledCall(globalObject, ProfilingReason::API, initializer, JSC::getCallData(initializer), globalObject->globalThis(), args);
if (auto* exception = scope.exception()) [[unlikely]] {
Expand Down
3 changes: 2 additions & 1 deletion src/jsc/bindings/BunProcessReportObjectWindows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,9 @@ JSValue constructReportObjectWindows(VM& vm, Zig::GlobalObject* globalObject, Pr
RETURN_IF_EXCEPTION(scope, {});

// Environment variables
report->putDirect(vm, Identifier::fromString(vm, "environmentVariables"_s), globalObject->processEnvObject(), 0);
JSObject* env = globalObject->processEnvObject();
RETURN_IF_EXCEPTION(scope, {});
report->putDirect(vm, Identifier::fromString(vm, "environmentVariables"_s), env, 0);

return report;
}
Expand Down
5 changes: 4 additions & 1 deletion src/jsc/bindings/ImportMetaObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,10 @@ JSC_DEFINE_CUSTOM_SETTER(jsImportMetaObjectSetter_require, (JSGlobalObject * jsG
JSC_DEFINE_CUSTOM_GETTER(jsImportMetaObjectGetter_env, (JSGlobalObject * jsGlobalObject, JSC::EncodedJSValue thisValue, PropertyName propertyName))
{
auto* globalObject = uncheckedDowncast<Zig::GlobalObject>(jsGlobalObject);
return JSValue::encode(globalObject->m_processEnvObject.getInitializedOnMainThread(globalObject));
auto scope = DECLARE_THROW_SCOPE(globalObject->vm());
JSObject* env = globalObject->processEnvObject();
RETURN_IF_EXCEPTION(scope, {});
return JSValue::encode(env);
}

static const HashTableValue ImportMetaObjectPrototypeValues[] = {
Expand Down
36 changes: 17 additions & 19 deletions src/jsc/bindings/JSEnvironmentVariableMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,7 @@ RefPtr<SharedEnvStore> ensureSharedEnvStoreForWorker(Zig::GlobalObject* globalOb
// Founding a new tree. processEnvObject() forces the lazy init so the OS
// environment is captured before the swap below.
JSObject* envObject = globalObject->processEnvObject();
RETURN_IF_EXCEPTION(scope, nullptr);
if (!envObject->staticPropertiesReified()) {
envObject->reifyAllStaticProperties(globalObject);
RETURN_IF_EXCEPTION(scope, nullptr);
Expand Down Expand Up @@ -979,7 +980,7 @@ RefPtr<SharedEnvStore> ensureSharedEnvStoreForWorker(Zig::GlobalObject* globalOb
return store;
}

JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject)
JSObject* createEnvironmentVariablesMap(Zig::GlobalObject* globalObject)
{
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
Expand All @@ -998,7 +999,7 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject)
}

JSArray* keyArray = constructEmptyArray(globalObject, nullptr, count);
RETURN_IF_EXCEPTION(scope, {});
RETURN_IF_EXCEPTION(scope, nullptr);
#else
auto* structure = JSEnvironmentVariableMap::createStructure(vm, globalObject, globalObject->objectPrototype());
JSC::JSObject* object = JSEnvironmentVariableMap::create(vm, structure);
Expand Down Expand Up @@ -1044,7 +1045,9 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject)
// We can't really trust that the OS gives us valid UTF-8
auto name = String::fromUTF8ReplacingInvalidSequences(std::span { chars, len });
#if OS(WINDOWS)
keyArray->putByIndexInline(globalObject, (unsigned)i, jsString(vm, name), false);
// Define, don't [[Set]]: `list` points into the env table until this loop ends, and a [[Set]] can run an indexed setter from Object.prototype.
keyArray->putDirectIndex(globalObject, (unsigned)i, jsString(vm, name));
RETURN_IF_EXCEPTION(scope, nullptr);
#endif
if (name == TZ) {
hasTZ = true;
Expand Down Expand Up @@ -1073,14 +1076,16 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject)
// CustomGetterSetter doesn't support indexed properties yet.
// This causes strange issues when the environment variable name is an integer.
if (chars[0] >= '0' && chars[0] <= '9') [[unlikely]] {
if (auto index = parseIndex(identifier)) {
if (parseIndex(identifier)) {
ZigString valueString = { nullptr, 0 };
ZigString nameStr = toZigString(name);
if (Bun__getEnvValue(globalObject, &nameStr, &valueString)) {
JSValue value = jsString(vm, Zig::toStringCopy(valueString));
RETURN_IF_EXCEPTION(scope, {});
object->putDirectIndex(globalObject, *index, value, 0, PutDirectIndexLikePutDirect);
RETURN_IF_EXCEPTION(scope, {});
RETURN_IF_EXCEPTION(scope, nullptr);
// The base define, not putDirectIndex: on this exotic object putDirectIndex ends in a [[Set]], which can run an indexed setter from Object.prototype while `list` is still in use.
PropertyDescriptor descriptor(value, 0);
JSObject::defineOwnProperty(object, globalObject, identifier, descriptor, false);
RETURN_IF_EXCEPTION(scope, nullptr);
}
continue;
}
Expand Down Expand Up @@ -1135,25 +1140,18 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject)
auto editWindowsEnvVar = JSC::JSFunction::create(vm, globalObject, 0, String("editWindowsEnvVar"_s), jsEditWindowsEnvVar, ImplementationVisibility::Public);

JSC::JSFunction* getSourceEvent = JSC::JSFunction::create(vm, globalObject, processObjectInternalsWindowsEnvCodeGenerator(vm), globalObject);
RETURN_IF_EXCEPTION(scope, {});
RETURN_IF_EXCEPTION(scope, nullptr);
JSC::MarkedArgumentBuffer args;
args.append(object);
args.append(keyArray);
args.append(editWindowsEnvVar);
args.append(JSC::JSFunction::create(vm, globalObject, 2, "coerceForWrite"_s, jsProcessEnvCoerceForWrite, ImplementationVisibility::Private));
args.append(JSC::JSFunction::create(vm, globalObject, 1, "resetForDelete"_s, jsProcessEnvResetForDelete, ImplementationVisibility::Private));
auto clientData = WebCore::clientData(vm);
JSC::CallData callData = JSC::getCallData(getSourceEvent);
NakedPtr<JSC::Exception> returnedException = nullptr;
auto result = JSC::profiledCall(globalObject, JSC::ProfilingReason::API, getSourceEvent, callData, globalObject->globalThis(), args, returnedException);
RETURN_IF_EXCEPTION(scope, {});

if (returnedException) {
throwException(globalObject, scope, returnedException.get());
return jsUndefined();
}

RELEASE_AND_RETURN(scope, result);
JSValue result = JSC::profiledCall(globalObject, JSC::ProfilingReason::API, getSourceEvent, callData, globalObject->globalThis(), args);
RETURN_IF_EXCEPTION(scope, nullptr);
// windowsEnv returns `new Proxy(...)`, so a normal return is always an object.
return asObject(result);
#else
return object;
#endif
Expand Down
3 changes: 2 additions & 1 deletion src/jsc/bindings/JSEnvironmentVariableMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ class JSEnvironmentVariableMap final : public JSC::JSNonFinalObject {
}
};

JSC::JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject);
// Null with an exception pending if it threw (Windows finishes the map in JS).
JSC::JSObject* createEnvironmentVariablesMap(Zig::GlobalObject* globalObject);

// Setting TZ must make *existing* Date instances recompute local time. JSC's DateCache
// reset only clears shared slots; live DateInstances keep a Ref to DateInstanceData
Expand Down
32 changes: 15 additions & 17 deletions src/jsc/bindings/JSPropertyIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,24 +56,22 @@ extern "C" JSPropertyIterator* Bun__JSPropertyIterator__create(JSC::JSGlobalObje
if (object->type() == JSC::ProxyObjectType) [[unlikely]] {
// Check if we're actually iterating through the JSEnvironmentVariableMap's proxy.
auto* zigGlobal = defaultGlobalObject(globalObject);
if (zigGlobal->m_processEnvObject.isInitialized()) {
if (object == zigGlobal->m_processEnvObject.get(zigGlobal)) {
object->methodTable()->getOwnPropertyNames(
object,
globalObject,
array,
DontEnumPropertiesMode::Exclude);
RETURN_IF_EXCEPTION(scope, nullptr);

*count = array.size();
if (array.size() == 0) {
return nullptr;
}

auto* iter = JSPropertyIterator::create(vm, array.releaseData());
iter->isSpecialProxy = true;
return iter;
if (object == zigGlobal->m_processEnvObject.get()) {
object->methodTable()->getOwnPropertyNames(
object,
globalObject,
array,
DontEnumPropertiesMode::Exclude);
RETURN_IF_EXCEPTION(scope, nullptr);

*count = array.size();
if (array.size() == 0) {
return nullptr;
}

auto* iter = JSPropertyIterator::create(vm, array.releaseData());
iter->isSpecialProxy = true;
return iter;
}
}
#endif
Expand Down
21 changes: 16 additions & 5 deletions src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2541,11 +2541,6 @@ void GlobalObject::finishCreation(VM& vm)
init.set(toJS(init.owner, globalObject, globalObject->performance().get()).getObject());
});

m_processEnvObject.initLater(
[](const JSC::LazyProperty<JSC::JSGlobalObject, JSC::JSObject>::Initializer& init) {
init.set(Bun::createEnvironmentVariablesMap(static_cast<Zig::GlobalObject*>(init.owner)).getObject());
});

m_processObject.initLater(
[](const JSC::LazyProperty<JSC::JSGlobalObject, Bun::Process>::Initializer& init) {
auto* globalObject = defaultGlobalObject(init.owner);
Expand Down Expand Up @@ -3086,6 +3081,22 @@ JSC_DEFINE_CUSTOM_GETTER(functionLazyNavigatorGetter,
return JSC::JSValue::encode(static_cast<Zig::GlobalObject*>(globalObject)->navigatorObject());
}

JSC::JSObject* GlobalObject::processEnvObject()
{
if (JSObject* env = m_processEnvObject.get())
return env;

auto& vm = this->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
JSObject* env = Bun::createEnvironmentVariablesMap(this);
RETURN_IF_EXCEPTION(scope, nullptr);
// Built re-entrantly if user code run by the Windows builtin read process.env; everyone must get that one.
if (JSObject* existing = m_processEnvObject.get())
return existing;
m_processEnvObject.set(vm, this, env);
return env;
}

JSC::GCClient::IsoSubspace* GlobalObject::subspaceForImpl(JSC::VM& vm)
{
return WebCore::subspaceForImpl<GlobalObject, WebCore::UseCustomHeapCellType::Yes>(
Expand Down
5 changes: 3 additions & 2 deletions src/jsc/bindings/ZigGlobalObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,8 @@ class GlobalObject : public Bun::GlobalScope {
void forbidExecution();

Bun::Process* processObject() const { return m_processObject.getInitializedOnMainThread(this); }
JSC::JSObject* processEnvObject() const { return m_processEnvObject.getInitializedOnMainThread(this); }
// Null with the exception pending if building it threw; nothing is cached then.
JSC::JSObject* processEnvObject();
JSC::JSObject* bunObject() const { return m_bunObject.getInitializedOnMainThread(this); }

uint8_t drainMicrotasks();
Expand Down Expand Up @@ -563,7 +564,7 @@ class GlobalObject : public Bun::GlobalScope {
\
V(public, Bun::JSMockModule, mockModule) \
\
V(public, LazyPropertyOfGlobalObject<JSObject>, m_processEnvObject) \
V(public, WriteBarrier<JSObject>, m_processEnvObject) \
\
V(public, LazyPropertyOfGlobalObject<Structure>, m_JSS3FileStructure) \
V(public, LazyPropertyOfGlobalObject<Structure>, m_S3ErrorStructure) \
Expand Down
5 changes: 3 additions & 2 deletions src/jsc/bindings/webcore/JSWorker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,9 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor::

if (envValue && envValue.isCell()) {
envObject = dynamicDowncast<JSC::JSObject>(envValue);
} else if (globalObject->m_processEnvObject.isInitialized()) {
envObject = globalObject->processEnvObject();
} else {
// Null (nothing to copy) unless process.env was ever built.
envObject = globalObject->m_processEnvObject.get();
}

if (envObject) {
Expand Down
Loading