diff --git a/packages/bun-internal-test/src/runner.node.mjs b/packages/bun-internal-test/src/runner.node.mjs index 14177e865d..5894de085b 100644 --- a/packages/bun-internal-test/src/runner.node.mjs +++ b/packages/bun-internal-test/src/runner.node.mjs @@ -43,7 +43,7 @@ function defaultConcurrency() { // Concurrency causes more flaky tests, only enable it by default on windows // See https://github.com/oven-sh/bun/issues/8071 if (windows) { - return Math.floor((cpus().length - 2) / 2); + return Math.floor((cpus().length - 2) / 3); } return 1; } diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index b8908fd373..6d5c13fe04 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -1710,21 +1710,17 @@ pub const VirtualMachine = struct { if (std.fs.path.isAbsolute(normalized_specifier)) { if (std.fs.path.dirname(normalized_specifier)) |dir| { // 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], - '/', - ); + break :name bun.strings.normalizeSlashesOnly(&specifier_cache_resolver_buf, dir, std.fs.path.sep); } } var parts = [_]string{ source_to_use, normalized_specifier, - bun.pathLiteral("../"), + bun.pathLiteral(".."), }; - break :name bun.path.joinAbsStringBufZTrailingSlash( + break :name bun.path.joinAbsStringBufZ( jsc_vm.bundler.fs.top_level_dir, &specifier_cache_resolver_buf, &parts, @@ -1732,9 +1728,11 @@ pub const VirtualMachine = struct { ); }; + // Only re-query if we previously had something cached. if (jsc_vm.bundler.resolver.bustDirCache(buster_name)) { continue; } + return error.ModuleNotFound; }, }; @@ -3446,7 +3444,7 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime // on windows we receive file events for all items affected by a directory change // so we only need to clear the directory cache. all other effects will be handled // by the file events - _ = resolver.bustDirCache(file_path); + _ = resolver.bustDirCache(strings.pathWithoutTrailingSlashOne(file_path)); continue; } var affected_buf: [128][]const u8 = undefined; @@ -3496,7 +3494,7 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime } } - _ = resolver.bustDirCache(file_path); + _ = resolver.bustDirCache(strings.pathWithoutTrailingSlashOne(file_path)); if (entries_option) |dir_ent| { var last_file_hash: GenericWatcher.HashType = std.math.maxInt(GenericWatcher.HashType); diff --git a/src/bundler.zig b/src/bundler.zig index b8f57233c6..c708222aa0 100644 --- a/src/bundler.zig +++ b/src/bundler.zig @@ -403,20 +403,16 @@ pub const Bundler = struct { if (std.fs.path.isAbsolute(entry_point)) { if (std.fs.path.dirname(entry_point)) |dir| { // Normalized with trailing slash - break :name bun.strings.normalizeSlashesOnly( - &cache_bust_buf, - if (dir.len == 1) dir else entry_point[0 .. dir.len + 1], - '/', - ); + break :name bun.strings.normalizeSlashesOnly(&cache_bust_buf, dir, std.fs.path.sep); } } var parts = [_]string{ entry_point, - bun.pathLiteral("../"), + bun.pathLiteral(".."), }; - break :name bun.path.joinAbsStringBufZTrailingSlash( + break :name bun.path.joinAbsStringBufZ( bundler.fs.top_level_dir, &cache_bust_buf, &parts, diff --git a/src/codegen/bundle-modules.ts b/src/codegen/bundle-modules.ts index 73d1b4c13e..0c5cff5ea9 100644 --- a/src/codegen/bundle-modules.ts +++ b/src/codegen/bundle-modules.ts @@ -336,7 +336,15 @@ if (!debug) { namespace Bun { namespace InternalModuleRegistryConstants { - ${moduleList.map((id, n) => declareASCIILiteral(`${idToEnumName(id)}Code`, outputs.get(id.slice(0, -3)))).join("\n")} + ${moduleList + .map((id, n) => { + const out = outputs.get(id.slice(0, -3).replaceAll("/", path.sep)); + if (!out) { + throw new Error(`Missing output for ${id}`); + } + return declareASCIILiteral(`${idToEnumName(id)}Code`, out); + }) + .join("\n")} } }`, ); diff --git a/src/codegen/generate-classes.ts b/src/codegen/generate-classes.ts index f8f5559671..df8057f7af 100644 --- a/src/codegen/generate-classes.ts +++ b/src/codegen/generate-classes.ts @@ -567,9 +567,6 @@ JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES ${name}::construct(JSC::JSGlobalObj obj.estimatedSize ? ` auto size = ${symbolName(typeName, "estimatedSize")}(ptr); -#if ASSERT_ENABLED - ASSERT(size > 0); -#endif vm.heap.reportExtraMemoryAllocated(instance, size);` : "" } @@ -1231,9 +1228,6 @@ void ${name}::visitChildrenImpl(JSCell* cell, Visitor& visitor) estimatedSize ? `if (auto* ptr = thisObject->wrapped()) { auto size = ${symbolName(typeName, "estimatedSize")}(ptr); -#if ASSERT_ENABLED - ASSERT(size > 0); -#endif visitor.reportExtraMemoryVisited(size); }` : "" @@ -1404,9 +1398,6 @@ extern "C" EncodedJSValue ${typeName}__create(Zig::GlobalObject* globalObject, v obj.estimatedSize ? ` auto size = ${symbolName(typeName, "estimatedSize")}(ptr); -#if ASSERT_ENABLED - ASSERT(size > 0); -#endif vm.heap.reportExtraMemoryAllocated(instance, size);` : "" } diff --git a/src/fs.zig b/src/fs.zig index fd0e0d76af..f38bd70de3 100644 --- a/src/fs.zig +++ b/src/fs.zig @@ -109,16 +109,11 @@ pub const FileSystem = struct { ENOTDIR, }; - pub fn init( - top_level_dir: ?string, - ) !*FileSystem { + pub fn init(top_level_dir: ?string) !*FileSystem { return initWithForce(top_level_dir, false); } - pub fn initWithForce( - top_level_dir_: ?string, - comptime force: bool, - ) !*FileSystem { + pub fn initWithForce(top_level_dir_: ?string, comptime force: bool) !*FileSystem { const allocator = bun.fs_allocator; var top_level_dir = top_level_dir_ orelse (if (Environment.isBrowser) "/project/" else try bun.getcwdAlloc(allocator)); @@ -134,9 +129,7 @@ pub const FileSystem = struct { if (!instance_loaded or force) { instance = FileSystem{ .top_level_dir = top_level_dir, - .fs = Implementation.init( - top_level_dir, - ), + .fs = Implementation.init(top_level_dir), // must always use default_allocator since the other allocators may not be threadsafe when an element resizes .dirname_store = DirnameStore.init(bun.default_allocator), .filename_store = FilenameStore.init(bun.default_allocator), @@ -1001,8 +994,18 @@ pub const FileSystem = struct { // https://twitter.com/jarredsumner/status/1655787337027309568 // https://twitter.com/jarredsumner/status/1655714084569120770 // https://twitter.com/jarredsumner/status/1655464485245845506 - pub fn readDirectoryWithIterator(fs: *RealFS, _dir: string, _handle: ?std.fs.Dir, generation: bun.Generation, store_fd: bool, comptime Iterator: type, iterator: Iterator) !*EntriesOption { - var dir = _dir; + pub fn readDirectoryWithIterator( + fs: *RealFS, + dir_maybe_trail_slash: string, + maybe_handle: ?std.fs.Dir, + generation: bun.Generation, + store_fd: bool, + comptime Iterator: type, + iterator: Iterator, + ) !*EntriesOption { + var dir = bun.strings.pathWithoutTrailingSlashOne(dir_maybe_trail_slash); + + bun.resolver.Resolver.assertValidCacheKey(dir); var cache_result: ?allocators.Result = null; if (comptime FeatureFlags.enable_entry_cache) { fs.entries_mutex.lock(); @@ -1028,20 +1031,20 @@ pub const FileSystem = struct { } } - var handle = _handle orelse try fs.openDir(dir); + var handle = maybe_handle orelse try fs.openDir(dir); defer { - if (_handle == null and (!store_fd or fs.needToCloseFiles())) { + if (maybe_handle == null and (!store_fd or fs.needToCloseFiles())) { handle.close(); } } // if we get this far, it's a real directory, so we can just store the dir name. - if (_handle == null) { + if (maybe_handle == null) { dir = try if (in_place) |existing| existing.dir else - DirnameStore.instance.append(string, _dir); + DirnameStore.instance.append(string, dir_maybe_trail_slash); } // Cache miss: read the directory entries diff --git a/src/resolver/resolve_path.zig b/src/resolver/resolve_path.zig index 36cb8a70c6..5f03cd8129 100644 --- a/src/resolver/resolve_path.zig +++ b/src/resolver/resolve_path.zig @@ -1085,10 +1085,10 @@ pub fn normalizeStringBuf( str: []const u8, buf: []u8, comptime allow_above_root: bool, - comptime _platform: Platform, + comptime platform: Platform, comptime preserve_trailing_slash: bool, ) []u8 { - return normalizeStringBufT(u8, str, buf, allow_above_root, _platform, preserve_trailing_slash); + return normalizeStringBufT(u8, str, buf, allow_above_root, platform, preserve_trailing_slash); } pub fn normalizeStringBufT( @@ -1096,12 +1096,10 @@ pub fn normalizeStringBufT( str: []const T, buf: []T, comptime allow_above_root: bool, - comptime _platform: Platform, + comptime platform: Platform, comptime preserve_trailing_slash: bool, ) []T { - const platform = comptime _platform.resolve(); - - switch (comptime platform) { + switch (comptime platform.resolve()) { .auto => @compileError("unreachable"), .windows => { diff --git a/src/resolver/resolver.zig b/src/resolver/resolver.zig index 0c65b7bfdc..587f87fa9d 100644 --- a/src/resolver/resolver.zig +++ b/src/resolver/resolver.zig @@ -552,8 +552,9 @@ pub const Resolver = struct { // do a lot of testing with various benchmarks and there aren't any regressions. mutex: *Mutex, - // This cache maps a directory path to information about that directory and - // all parent directories + /// This cache maps a directory path to information about that directory and + /// all parent directories. When interacting with this structure, make sure + /// to validate your keys with `Resolver.assertValidCacheKey` dir_cache: *DirInfo.HashMap, /// This is set to false for the runtime. The runtime should choose "main" @@ -1139,9 +1140,7 @@ pub const Resolver = struct { // experience unexpected build failures later on other operating systems. // Treating these paths as absolute paths on all platforms means Windows // users will not be able to accidentally make use of these paths. - if (std.fs.path.isAbsolutePosix(import_path) or - (if (Environment.isWindows) std.fs.path.isAbsoluteWindows(import_path) else false)) - { + if (std.fs.path.isAbsolute(import_path)) { if (r.debug_logs) |*debug| { debug.addNoteFmt("The import \"{s}\" is being treated as an absolute path", .{import_path}); } @@ -1582,17 +1581,43 @@ pub const Resolver = struct { const dev = Output.scoped(.Resolver, false); - /// Bust the directory cache for the given path. - /// Returns `true` if something was deleted, otherwise `false`. - pub fn bustDirCache(r: *ThisResolver, path: string) bool { - dev("Bust {s}", .{path}); + /// Directory cache keys must follow the following rules. If the rules are broken, + /// then there will be conflicting cache entries, and trying to bust the cache may not work. + /// + /// When an incorrect cache key is used, this assertion will trip; ignoring it allows + /// very very subtle cache invalidation issues to happen, which will cause modules to + /// mysteriously fail to resolve. + /// + /// The rules for this changed in https://github.com/oven-sh/bun/pull/9144 after multiple + /// cache issues were found on Windows. These issues extended to other platforms because + /// we never checked if the cache key was following the rules. + /// + /// CACHE KEY RULES: + /// A cache key must use native slashes, and must NOT end with a trailing slash. + /// But drive roots MUST have a trailing slash ('/' and 'C:\') + /// UNC paths, even if the root, must not have the trailing slash. + /// + /// The helper function bun.strings.pathWithoutTrailingSlashOne can be used to remove + /// the trailing slash from a path, but also note it will only remove a SINGLE slash. + pub fn assertValidCacheKey(path: []const u8) void { if (Environment.allow_assert) { - if (path[path.len - 1] != '/') { - std.debug.panic("Expected a trailing slash on {s}", .{path}); + if (path.len > 1 and strings.charIsAnySlash(path[path.len - 1]) and !if (Environment.isWindows) + path.len == 3 and path[1] == ':' + else + path.len == 1) + { + std.debug.panic("Internal Assertion Failure: Invalid cache key \"{s}\"\nSee Resolver.assertValidCacheKey for details.", .{path}); } } + } + + /// Bust the directory cache for the given path. + /// See `assertValidCacheKey` for requirements on the input + pub fn bustDirCache(r: *ThisResolver, path: string) bool { + assertValidCacheKey(path); const first_bust = r.fs.fs.bustEntriesCache(path); const second_bust = r.dir_cache.remove(path); + dev("Bust {s} = {}, {}", .{ path, first_bust, second_bust }); return first_bust or second_bust; } @@ -2050,18 +2075,21 @@ pub const Resolver = struct { } fn dirInfoForResolution( r: *ThisResolver, - dir_path: string, + dir_path_maybe_trail_slash: string, package_id: Install.PackageID, ) !?*DirInfo { std.debug.assert(r.package_manager != null); - var dir_cache_info_result = r.dir_cache.getOrPut(dir_path) catch unreachable; + const dir_path = strings.pathWithoutTrailingSlashOne(dir_path_maybe_trail_slash); + + assertValidCacheKey(dir_path); + var dir_cache_info_result = r.dir_cache.getOrPut(dir_path) catch bun.outOfMemory(); if (dir_cache_info_result.status == .exists) { // we've already looked up this package before return r.dir_cache.atIndex(dir_cache_info_result.index).?; } var rfs = &r.fs.fs; - var cached_dir_entry_result = rfs.entries.getOrPut(dir_path) catch unreachable; + var cached_dir_entry_result = rfs.entries.getOrPut(dir_path) catch bun.outOfMemory(); var dir_entries_option: *Fs.FileSystem.RealFS.EntriesOption = undefined; var needs_iter = true; @@ -2510,13 +2538,6 @@ pub const Resolver = struct { return r.dirInfoCachedMaybeLog(path, false, true) catch null; } - pub inline fn readDirInfoCacheOnly( - r: *ThisResolver, - path: string, - ) ?*DirInfo { - return r.dir_cache.get(path); - } - inline fn isDotSlash(path: string) bool { return switch (Environment.os) { else => strings.eqlComptime(path, "./"), @@ -2537,7 +2558,9 @@ pub const Resolver = struct { _path = r.fs.normalizeBuf(&win32_normalized_dir_info_cache_buf, _path); } - const top_result = try r.dir_cache.getOrPut(_path); + const path_without_trailing_slash = strings.pathWithoutTrailingSlashOne(_path); + assertValidCacheKey(path_without_trailing_slash); + const top_result = try r.dir_cache.getOrPut(path_without_trailing_slash); if (top_result.status != .unknown) { return r.dir_cache.atIndex(top_result.index); } @@ -2548,7 +2571,7 @@ pub const Resolver = struct { bun.copy(u8, dir_info_uncached_path_buf, _path); var path = dir_info_uncached_path_buf[0.._path.len]; - bufs(.dir_entry_paths_to_resolve)[0] = (DirEntryResolveQueueItem{ .result = top_result, .unsafe_path = path, .safe_path = "" }); + bufs(.dir_entry_paths_to_resolve)[0] = DirEntryResolveQueueItem{ .result = top_result, .unsafe_path = path, .safe_path = "" }; var top = Dirname.dirname(path); var top_parent: allocators.Result = allocators.Result{ @@ -2562,7 +2585,9 @@ pub const Resolver = struct { // we cannot just use "/" // we will write to the buffer past the ptr len so it must be a non-const buffer path[0..1]; - var rfs: *Fs.FileSystem.RealFS = &r.fs.fs; + assertValidCacheKey(root_path); + + const rfs = &r.fs.fs; rfs.entries_mutex.lock(); defer rfs.entries_mutex.unlock(); @@ -2612,8 +2637,6 @@ pub const Resolver = struct { // When this function halts, any item not processed means it's not found. defer { - - // Anything if (open_dir_count > 0 and (!r.store_fd or r.fs.fs.needToCloseFiles())) { const open_dirs: []std.fs.Dir = bufs(.open_dirs)[0..open_dir_count]; for (open_dirs) |*open_dir| { @@ -3302,7 +3325,7 @@ pub const Resolver = struct { defer arena.deinit(); var stack_fallback_allocator = std.heap.stackFallback(1024, arena.allocator()); - if (r.readDirInfo(strings.withoutTrailingSlash(str)) catch null) |result| { + if (r.readDirInfo(str) catch null) |result| { var dir_info = result; while (true) { @@ -3629,7 +3652,7 @@ pub const Resolver = struct { } } - const dir_path = Dirname.dirname(path); + const dir_path = bun.strings.pathWithoutTrailingSlashOne(Dirname.dirname(path)); const dir_entry: *Fs.FileSystem.RealFS.EntriesOption = rfs.readDirectory( dir_path, diff --git a/src/string_immutable.zig b/src/string_immutable.zig index d5d07c063f..ddec5616c0 100644 --- a/src/string_immutable.zig +++ b/src/string_immutable.zig @@ -742,6 +742,20 @@ pub fn withoutTrailingSlashWindowsPath(this: string) []const u8 { return href; } +/// This will remove ONE trailing slash at the end of a string, +/// but on Windows it will not remove the \ in "C:\" +pub fn pathWithoutTrailingSlashOne(str: []const u8) []const u8 { + return if (str.len > 0 and charIsAnySlash(str[str.len - 1])) + if (Environment.isWindows and str.len == 3 and str[1] == ':') + // Preserve "C:\" + str + else + // Remove one slash + str[0 .. str.len - 1] + else + str; +} + pub fn withoutLeadingSlash(this: string) []const u8 { return std.mem.trimLeft(u8, this, "/"); } diff --git a/test/js/deno/fetch/body.test.ts b/test/js/deno/fetch/body.test.ts index b2c986ffdf..b1e6118570 100644 --- a/test/js/deno/fetch/body.test.ts +++ b/test/js/deno/fetch/body.test.ts @@ -36,7 +36,7 @@ test({ net: true } }, async function bodyMultipartFormData() { - const response = await fetch("http://localhost:4545/multipart_form_data.txt"); + const response = await fetch("http://localhost:" + PORT + "/multipart_form_data.txt"); assert(response.body instanceof ReadableStream); const text = await response.text(); const body = buildBody(text, response.headers); @@ -51,7 +51,7 @@ test({ net: true } }, async function bodyURLEncodedFormData() { - const response = await fetch("http://localhost:4545/subdir/form_urlencoded.txt"); + const response = await fetch("http://localhost:" + PORT + "/subdir/form_urlencoded.txt"); assert(response.body instanceof ReadableStream); const text = await response.text(); const body = buildBody(text, response.headers); diff --git a/test/js/deno/harness.ts b/test/js/deno/harness.ts index 89caad0ee5..8bca01b3ba 100644 --- a/test/js/deno/harness.ts +++ b/test/js/deno/harness.ts @@ -29,7 +29,7 @@ export function createDenoTest(path: string) { beforeAll(() => { server = serve({ - port: 4545, + port: 0, fetch(request: Request): Response { const { url } = request; const { pathname, search } = new URL(url); @@ -40,6 +40,7 @@ export function createDenoTest(path: string) { return Response.redirect(target.toString()); }, }); + globalThis.PORT = server.port; }); afterAll(() => { diff --git a/test/regression/issue/00631.test.ts b/test/regression/issue/00631.test.ts index c9303b5c17..643f5ed98f 100644 --- a/test/regression/issue/00631.test.ts +++ b/test/regression/issue/00631.test.ts @@ -22,6 +22,9 @@ it("JSON strings escaped properly", async () => { env: bunEnv, cwd: testDir, }); + if (exitCode !== 0) { + console.log(stderr.toString("utf8")); + } expect(exitCode).toBe(0); const packageContents = readFileSync(join(testDir, "package.json"), { encoding: "utf8" }); diff --git a/test/regression/issue/03216.test.ts b/test/regression/issue/03216.test.ts index 5eced0bf9e..ffa3c53c3c 100644 --- a/test/regression/issue/03216.test.ts +++ b/test/regression/issue/03216.test.ts @@ -10,12 +10,14 @@ test("runtime directory caching gets invalidated", () => { writeFileSync( join(tmp, "index.ts"), `const file = \`\${import.meta.dir}/temp.mjs\`; +const file2 = \`\${import.meta.dir}/second.mjs\`; import { existsSync, unlinkSync, writeFileSync } from "fs"; -if (existsSync(file)) { +if (existsSync(file) || existsSync(file2)) { console.log("temp.mjs cannot exist before running this script"); - unlinkSync(file); + try { unlinkSync(file); } catch {} + try { unlinkSync(file2); } catch {} process.exit(2); } @@ -27,6 +29,15 @@ try { } finally { unlinkSync(file); } + +writeFileSync(file2, "export default 2;"); + +try { + const module = await import(file2); + console.log(module.default); +} finally { + unlinkSync(file2); +} `, ); @@ -36,6 +47,10 @@ try { env: bunEnv, }); + if (result.exitCode !== 0) { + console.log(result.stderr.toString("utf-8")); + } + expect(result.exitCode).toBe(0); - expect(result.stdout.toString("utf-8")).toBe("1\n"); + expect(result.stdout.toString("utf-8")).toBe("1\n2\n"); });