From c54844b30b2fff87728bc13da32fdc6f899b5b57 Mon Sep 17 00:00:00 2001 From: dave caruso Date: Tue, 27 Feb 2024 16:30:34 -0800 Subject: [PATCH] windows: random things (#9046) * random things * fix reliability of loading napi stuff * fix posix build * a --- src/bun.js/RuntimeTranspilerCache.zig | 1 - src/bun.js/bindings/BunProcess.cpp | 15 +++++--- src/bun.js/bindings/exports.zig | 25 +++++++++++++ src/bun.js/javascript.zig | 10 ++++-- src/bundler.zig | 10 ++++-- .../builtins/ReadableByteStreamInternals.ts | 3 ++ src/string.zig | 2 +- src/string_immutable.zig | 35 +++++++++++++------ src/windows.zig | 2 ++ test/cli/hot/hot.test.ts | 7 +--- test/cli/run/require-cache.test.ts | 25 +++++++------ 11 files changed, 97 insertions(+), 38 deletions(-) diff --git a/src/bun.js/RuntimeTranspilerCache.zig b/src/bun.js/RuntimeTranspilerCache.zig index 0bc5536c6a..bac64f6d8f 100644 --- a/src/bun.js/RuntimeTranspilerCache.zig +++ b/src/bun.js/RuntimeTranspilerCache.zig @@ -185,7 +185,6 @@ pub const RuntimeTranspilerCache = struct { .utf8 => Encoding.utf8, .utf16 => Encoding.utf16, .latin1 => Encoding.latin1, - else => @panic("Unexpected encoding"), }, }, .sourcemap_byte_length = sourcemap.len, diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index b6dc6d4e96..8330841b78 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -238,8 +238,10 @@ JSC_DEFINE_CUSTOM_SETTER(Process_defaultSetter, } extern "C" bool Bun__resolveEmbeddedNodeFile(void*, BunString*); +#if OS(WINDOWS) +extern "C" HMODULE Bun__LoadLibraryBunString(BunString*); +#endif -JSC_DECLARE_HOST_FUNCTION(Process_functionDlopen); JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen, (JSC::JSGlobalObject * globalObject_, JSC::CallFrame* callFrame)) { @@ -291,8 +293,8 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen, RETURN_IF_EXCEPTION(scope, {}); #if OS(WINDOWS) - CString utf8 = filename.utf8(); - HMODULE handle = LoadLibraryA(utf8.data()); + BunString filename_str = Bun::toString(filename); + HMODULE handle = Bun__LoadLibraryBunString(&filename_str); #else CString utf8 = filename.utf8(); void* handle = dlopen(utf8.data(), RTLD_LAZY); @@ -300,7 +302,12 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen, if (!handle) { #if OS(WINDOWS) - WTF::String msg = makeString("LoadLibraryA failed with error code: "_s, GetLastError()); + DWORD errorId = GetLastError(); + LPWSTR messageBuffer = nullptr; + size_t size = FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, errorId, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPWSTR)&messageBuffer, 0, NULL); + WTF::String msg = makeString("LoadLibrary failed: ", WTF::StringView((UCHAR*)messageBuffer, size)); + LocalFree(messageBuffer); #else WTF::String msg = WTF::String::fromUTF8(dlerror()); #endif diff --git a/src/bun.js/bindings/exports.zig b/src/bun.js/bindings/exports.zig index 12f9657302..83c0e68d00 100644 --- a/src/bun.js/bindings/exports.zig +++ b/src/bun.js/bindings/exports.zig @@ -924,5 +924,30 @@ comptime { TestScope.shim.ref(); BodyValueBuffererContext.shim.ref(); + + _ = Bun__LoadLibraryBunString; } } + +/// Returns null on error. Use windows API to lookup the actual error. +/// The reason this function is in zig is so that we can use our own utf16-conversion functions. +/// +/// Using characters16() does not seem to always have the sentinel. or something else +/// broke when I just used it. Not sure. ... but this works! +pub export fn Bun__LoadLibraryBunString(str: *bun.String) ?*anyopaque { + var buf: bun.WPathBuffer = undefined; + const data = switch (str.encoding()) { + .utf8 => bun.strings.convertUTF8toUTF16InBuffer(&buf, str.utf8()), + .utf16 => brk: { + @memcpy(buf[0..str.length()], str.utf16()); + break :brk buf[0..str.length()]; + }, + .latin1 => brk: { + bun.strings.copyU8IntoU16(&buf, str.latin1()); + break :brk buf[0..str.length()]; + }, + }; + buf[data.len] = 0; + const LOAD_WITH_ALTERED_SEARCH_PATH = 0x00000008; + return bun.windows.LoadLibraryExW(buf[0..data.len :0].ptr, null, LOAD_WITH_ALTERED_SEARCH_PATH); +} diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index 5e0c45a8fe..b8908fd373 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -1709,15 +1709,19 @@ pub const VirtualMachine = struct { const buster_name = name: { if (std.fs.path.isAbsolute(normalized_specifier)) { if (std.fs.path.dirname(normalized_specifier)) |dir| { - // With trailing slash - break :name if (dir.len == 1) dir else normalized_specifier[0 .. dir.len + 1]; + // Normalized with trailing slash + break :name bun.strings.normalizeSlashesOnly( + &specifier_cache_resolver_buf, + if (dir.len == 1) dir else normalized_specifier[0 .. dir.len + 1], + '/', + ); } } var parts = [_]string{ source_to_use, normalized_specifier, - "../", + bun.pathLiteral("../"), }; break :name bun.path.joinAbsStringBufZTrailingSlash( diff --git a/src/bundler.zig b/src/bundler.zig index 26939d5e74..b8f57233c6 100644 --- a/src/bundler.zig +++ b/src/bundler.zig @@ -402,14 +402,18 @@ pub const Bundler = struct { const buster_name = name: { if (std.fs.path.isAbsolute(entry_point)) { if (std.fs.path.dirname(entry_point)) |dir| { - // With trailing slash - break :name if (dir.len == 1) dir else entry_point[0 .. dir.len + 1]; + // Normalized with trailing slash + break :name bun.strings.normalizeSlashesOnly( + &cache_bust_buf, + if (dir.len == 1) dir else entry_point[0 .. dir.len + 1], + '/', + ); } } var parts = [_]string{ entry_point, - "../", + bun.pathLiteral("../"), }; break :name bun.path.joinAbsStringBufZTrailingSlash( diff --git a/src/js/builtins/ReadableByteStreamInternals.ts b/src/js/builtins/ReadableByteStreamInternals.ts index 1f1853ef4b..381f7201b2 100644 --- a/src/js/builtins/ReadableByteStreamInternals.ts +++ b/src/js/builtins/ReadableByteStreamInternals.ts @@ -222,7 +222,9 @@ export function readableByteStreamControllerPull(controller) { } export function readableByteStreamControllerShouldCallPull(controller) { + $assert(controller); const stream = $getByIdDirectPrivate(controller, "controlledReadableStream"); + $assert(stream); if ($getByIdDirectPrivate(stream, "state") !== $streamReadable) return false; if ($getByIdDirectPrivate(controller, "closeRequested")) return false; @@ -244,6 +246,7 @@ export function readableByteStreamControllerShouldCallPull(controller) { } export function readableByteStreamControllerCallPullIfNeeded(controller) { + $assert(controller); if (!$readableByteStreamControllerShouldCallPull(controller)) return; if ($getByIdDirectPrivate(controller, "pulling")) { diff --git a/src/string.zig b/src/string.zig index 8795f788c7..0cea9d4643 100644 --- a/src/string.zig +++ b/src/string.zig @@ -672,7 +672,7 @@ pub const String = extern struct { return ""; } - pub fn encoding(self: String) bun.strings.Encoding { + pub fn encoding(self: String) bun.strings.EncodingNonAscii { if (self.isUTF16()) { return .utf16; } diff --git a/src/string_immutable.zig b/src/string_immutable.zig index 62cf83aa8f..d5d07c063f 100644 --- a/src/string_immutable.zig +++ b/src/string_immutable.zig @@ -17,6 +17,13 @@ pub const Encoding = enum { utf16, }; +/// Returned by classification functions that do not discriminate between utf8 and ascii. +pub const EncodingNonAscii = enum { + utf8, + utf16, + latin1, +}; + pub inline fn containsChar(self: string, char: u8) bool { return indexOfChar(self, char) != null; } @@ -1774,17 +1781,8 @@ pub fn toWPathNormalizeAutoExtend(wbuf: []u16, utf8: []const u8) [:0]const u16 { pub fn toWPathNormalized(wbuf: []u16, utf8: []const u8) [:0]const u16 { var renormalized: [bun.MAX_PATH_BYTES]u8 = undefined; - var path_to_use = utf8; - if (bun.strings.containsChar(utf8, '/')) { - @memcpy(renormalized[0..utf8.len], utf8); - for (renormalized[0..utf8.len]) |*c| { - if (c.* == '/') { - c.* = '\\'; - } - } - path_to_use = renormalized[0..utf8.len]; - } + var path_to_use = normalizeSlashesOnly(&renormalized, utf8, '\\'); // is there a trailing slash? Let's remove it before converting to UTF-16 if (path_to_use.len > 3 and bun.path.isSepAny(path_to_use[path_to_use.len - 1])) { @@ -1794,6 +1792,23 @@ pub fn toWPathNormalized(wbuf: []u16, utf8: []const u8) [:0]const u16 { return toWPath(wbuf, path_to_use); } +pub fn normalizeSlashesOnly(buf: []u8, utf8: []const u8, comptime desired_slash: u8) []const u8 { + comptime std.debug.assert(desired_slash == '/' or desired_slash == '\\'); + const undesired_slash = if (desired_slash == '/') '\\' else '/'; + + if (bun.strings.containsChar(utf8, undesired_slash)) { + @memcpy(buf[0..utf8.len], utf8); + for (buf[0..utf8.len]) |*c| { + if (c.* == undesired_slash) { + c.* = desired_slash; + } + } + return buf[0..utf8.len]; + } + + return utf8; +} + pub fn toWDirNormalized(wbuf: []u16, utf8: []const u8) [:0]const u16 { var renormalized: [bun.MAX_PATH_BYTES]u8 = undefined; var path_to_use = utf8; diff --git a/src/windows.zig b/src/windows.zig index dcd57e1401..b605f37234 100644 --- a/src/windows.zig +++ b/src/windows.zig @@ -2985,6 +2985,8 @@ pub extern fn LoadLibraryA( [*:0]const u8, ) ?*anyopaque; +pub extern fn LoadLibraryExW([*:0]const u16, ?HANDLE, DWORD) ?*anyopaque; + pub extern "kernel32" fn CreateHardLinkW( newFileName: LPCWSTR, existingFileName: LPCWSTR, diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index 37674913eb..cdbbb3ac6e 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -156,14 +156,9 @@ it("should not hot reload when a random file is written", async () => { writeFileSync(root + ".another.yet.js", code); unlinkSync(root + ".another.yet.js"); } - var waiter = new Promise((resolve, reject) => { - setTimeout(async () => { - resolve(); - }, 50); - }); var finished = false; await Promise.race([ - waiter, + Bun.sleep(200), (async () => { if (finished) { return; diff --git a/test/cli/run/require-cache.test.ts b/test/cli/run/require-cache.test.ts index e55502b9be..1d79577c7b 100644 --- a/test/cli/run/require-cache.test.ts +++ b/test/cli/run/require-cache.test.ts @@ -1,5 +1,5 @@ import { test, expect, describe } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, isWindows } from "harness"; import { join } from "path"; // This also tests __dirname and __filename @@ -63,14 +63,19 @@ describe("files transpiled and loaded don't leak file paths", () => { expect(exitCode).toBe(0); }, 30000); - test("via import()", () => { - const { stdout, exitCode } = Bun.spawnSync({ - cmd: [bunExe(), "--smol", "run", join(import.meta.dir, "esm-fixture-leak-small.mjs")], - env: bunEnv, - stderr: "inherit", - }); + test( + "via import()", + () => { + const { stdout, exitCode } = Bun.spawnSync({ + cmd: [bunExe(), "--smol", "run", join(import.meta.dir, "esm-fixture-leak-small.mjs")], + env: bunEnv, + stderr: "inherit", + }); - expect(stdout.toString().trim()).toEndWith("--pass--"); - expect(exitCode).toBe(0); - }, 30000); + expect(stdout.toString().trim()).toEndWith("--pass--"); + expect(exitCode).toBe(0); + }, + // TODO: Investigate why this is so slow on Windows + isWindows ? 60000 : 30000, + ); });