diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index 24331d5f9a..4752d5cfc8 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -69,6 +69,7 @@ #include #include #include +#include #else #include #include @@ -361,6 +362,9 @@ static char* toFileURI(std::span span) extern "C" size_t Bun__process_dlopen_count; +// "Fire and forget" wrapper around unlink for c usage that handles EINTR +extern "C" void Bun__unlink(const char*, size_t); + extern "C" void CrashHandler__setDlOpenAction(const char* action); extern "C" bool Bun__VM__allowAddons(void* vm); @@ -408,11 +412,13 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen, (JSC::JSGlobalObject * globalOb Strong strongModule = { vm, moduleObject }; WTF::String filename = callFrame->uncheckedArgument(1).toWTFString(globalObject); - if (filename.isEmpty()) { + + if (filename.isEmpty() && !scope.exception()) { JSC::throwTypeError(globalObject, scope, "dlopen requires a non-empty string as the second argument"_s); - return {}; } + RETURN_IF_EXCEPTION(scope, {}); + if (filename.startsWith("file://"_s)) { WTF::URL fileURL = WTF::URL(filename); if (!fileURL.isValid() || !fileURL.protocolIsFile()) { @@ -423,6 +429,8 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen, (JSC::JSGlobalObject * globalOb filename = fileURL.fileSystemPath(); } + CString utf8; + // Support embedded .node files // See StandaloneModuleGraph.zig for what this "$bunfs" thing is #if OS(WINDOWS) @@ -430,22 +438,73 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen, (JSC::JSGlobalObject * globalOb #else #define StandaloneModuleGraph__base_path "/$bunfs/"_s #endif + bool deleteAfter = false; if (filename.startsWith(StandaloneModuleGraph__base_path)) { BunString bunStr = Bun::toString(filename); if (Bun__resolveEmbeddedNodeFile(globalObject->bunVM(), &bunStr)) { filename = bunStr.toWTFString(BunString::ZeroCopy); + deleteAfter = !filename.startsWith("/proc/"_s); } } RETURN_IF_EXCEPTION(scope, {}); + + // For bun build --compile, we copy the .node file to a temp directory. + // It's best to delete it as soon as we can. + // https://github.com/oven-sh/bun/issues/19550 + const auto tryToDeleteIfNecessary = [&]() { +#if OS(WINDOWS) + if (deleteAfter) { + // Only call it once + deleteAfter = false; + if (filename.is8Bit()) { + filename.convertTo16Bit(); + } + + // Convert to 16-bit with a sentinel zero value. + auto span = filename.span16(); + auto dupeZ = new wchar_t[span.size() + 1]; + if (dupeZ) { + memcpy(dupeZ, span.data(), span.size_bytes()); + dupeZ[span.size()] = L'\0'; + + // We can't immediately delete the file on Windows. + // Instead, we mark it for deletion on reboot. + MoveFileExW( + dupeZ, + NULL, // NULL destination means delete + MOVEFILE_DELAY_UNTIL_REBOOT); + delete[] dupeZ; + } + } +#else + if (deleteAfter) { + deleteAfter = false; + Bun__unlink(utf8.data(), utf8.length()); + } +#endif + }; + + { + auto utf8_filename = filename.tryGetUTF8(ConversionMode::LenientConversion); + if (UNLIKELY(!utf8_filename)) { + JSC::throwTypeError(globalObject, scope, "process.dlopen requires a valid UTF-8 string for the filename"_s); + return {}; + } + utf8 = *utf8_filename; + } + #if OS(WINDOWS) BunString filename_str = Bun::toString(filename); HMODULE handle = Bun__LoadLibraryBunString(&filename_str); + +// On Windows, we use GetLastError() for error messages, so we can only delete after checking for errors #else - CString utf8 = filename.utf8(); CrashHandler__setDlOpenAction(utf8.data()); void* handle = dlopen(utf8.data(), RTLD_LAZY); CrashHandler__setDlOpenAction(nullptr); + + tryToDeleteIfNecessary(); #endif globalObject->m_pendingNapiModuleDlopenHandle = handle; @@ -482,12 +541,19 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen, (JSC::JSGlobalObject * globalOb WTF::String msg = errorBuilder.toString(); if (messageBuffer) LocalFree(messageBuffer); // Free the buffer allocated by FormatMessageW + + // Since we're relying on LastError(), we have to delete after checking for errors + tryToDeleteIfNecessary(); #else WTF::String msg = WTF::String::fromUTF8(dlerror()); #endif return throwError(globalObject, scope, ErrorCode::ERR_DLOPEN_FAILED, msg); } +#if OS(WINDOWS) + tryToDeleteIfNecessary(); +#endif + if (callCountAtStart != globalObject->napiModuleRegisterCallCount) { JSValue resultValue = globalObject->m_pendingNapiModuleAndExports[0].get(); globalObject->napiModuleRegisterCallCount = 0; @@ -528,7 +594,9 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen, (JSC::JSGlobalObject * globalOb dlclose(handle); #endif - JSC::throwTypeError(globalObject, scope, "symbol 'napi_register_module_v1' not found in native module. Is this a Node API (napi) module?"_s); + if (LIKELY(!scope.exception())) { + JSC::throwTypeError(globalObject, scope, "symbol 'napi_register_module_v1' not found in native module. Is this a Node API (napi) module?"_s); + } return {}; } @@ -542,7 +610,7 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen, (JSC::JSGlobalObject * globalOb EncodedJSValue exportsValue = JSC::JSValue::encode(exports); - char* filename_cstr = toFileURI(filename.utf8().span()); + char* filename_cstr = toFileURI(utf8.span()); napi_module nmodule { .nm_version = module_version, diff --git a/src/sys.zig b/src/sys.zig index dab7397648..5f142004c7 100644 --- a/src/sys.zig +++ b/src/sys.zig @@ -4611,6 +4611,11 @@ export fn Bun__errnoName(err: c_int) ?[*:0]const u8 { return @tagName(SystemErrno.init(err) orelse return null); } +/// Small "fire and forget" wrapper around unlink for c usage that handles EINTR, windows path conversion, etc. +export fn Bun__unlink(ptr: [*:0]const u8, len: usize) void { + _ = unlink(ptr[0..len :0]); +} + // TODO: this is wrong on Windows const libc_stat = bun.Stat; const Stat = std.fs.File.Stat; diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index f4905cd6af..c5c2169f37 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -1,5 +1,6 @@ import { spawnSync } from "bun"; import { beforeAll, describe, expect, it } from "bun:test"; +import { readdirSync } from "fs"; import { bunEnv, bunExe, tempDirWithFiles } from "harness"; import { join } from "path"; @@ -90,18 +91,23 @@ describe("napi", () => { stderr: "inherit", }); expect(build.success).toBeTrue(); - + const tmpdir = tempDirWithFiles("should-be-empty-except", {}); const result = spawnSync({ cmd: [exe, "self"], - env: bunEnv, + env: { ...bunEnv, BUN_TMPDIR: tmpdir }, stdin: "inherit", stderr: "inherit", stdout: "pipe", }); const stdout = result.stdout.toString().trim(); - expect(stdout).toBe("hello world!"); expect(result.success).toBeTrue(); + if (process.platform !== "win32") { + expect(readdirSync(tmpdir), "bun should clean up .node files").toBeEmpty(); + } else { + // On Windows, we have to mark it for deletion on reboot. + // Not clear how to test for that. + } }, 10 * 1000, );