From 644c5c449adc4222adcca1999344014fa7805bf7 Mon Sep 17 00:00:00 2001 From: dave caruso Date: Fri, 26 Jul 2024 19:39:37 -0700 Subject: [PATCH] fix a bundler crash (#12864) Co-authored-by: paperdave --- src/bundler/bundle_v2.zig | 25 ++++++++++++-------- src/thread_pool.zig | 8 ++----- src/work_pool.zig | 14 +++++++++-- test/bundler/bun-build-api.test.ts | 37 ++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 18 deletions(-) diff --git a/src/bundler/bundle_v2.zig b/src/bundler/bundle_v2.zig index 8387da17605d2d..f91d179556f2a0 100644 --- a/src/bundler/bundle_v2.zig +++ b/src/bundler/bundle_v2.zig @@ -1658,17 +1658,11 @@ pub const BundleV2 = struct { bundler.resolver.opts = bundler.options; - var this = try BundleV2.init(bundler, allocator, JSC.AnyEventLoop.init(allocator), false, JSC.WorkPool.get(), heap); + const this = try BundleV2.init(bundler, allocator, JSC.AnyEventLoop.init(allocator), false, JSC.WorkPool.get(), heap); this.plugins = completion.plugins; this.completion = completion; completion.bundler = this; - errdefer { - var out_log = Logger.Log.init(bun.default_allocator); - this.bundler.log.appendToWithRecycled(&out_log, true) catch bun.outOfMemory(); - completion.log = out_log; - } - defer { if (this.graph.pool.pool.threadpool_context == @as(?*anyopaque, @ptrCast(this.graph.pool))) { this.graph.pool.pool.threadpool_context = null; @@ -1678,6 +1672,16 @@ pub const BundleV2 = struct { this.deinit(); } + errdefer { + // Wait for wait groups to finish. There still may be + this.linker.source_maps.line_offset_wait_group.wait(); + this.linker.source_maps.quoted_contents_wait_group.wait(); + + var out_log = Logger.Log.init(bun.default_allocator); + this.bundler.log.appendToWithRecycled(&out_log, true) catch bun.outOfMemory(); + completion.log = out_log; + } + completion.result = .{ .value = .{ .output_files = try this.runFromJSInNewThread(config), @@ -3836,7 +3840,7 @@ pub const LinkerContext = struct { options: LinkerOptions = .{}, - wait_group: ThreadPoolLib.WaitGroup = undefined, + wait_group: ThreadPoolLib.WaitGroup = .{}, ambiguous_result_pool: std.ArrayList(MatchImport) = undefined, @@ -3874,10 +3878,10 @@ pub const LinkerContext = struct { }; pub const SourceMapData = struct { - line_offset_wait_group: sync.WaitGroup = undefined, + line_offset_wait_group: sync.WaitGroup = .{}, line_offset_tasks: []Task = &.{}, - quoted_contents_wait_group: sync.WaitGroup = undefined, + quoted_contents_wait_group: sync.WaitGroup = .{}, quoted_contents_tasks: []Task = &.{}, pub const Task = struct { @@ -9123,6 +9127,7 @@ pub const LinkerContext = struct { wait_group.deinit(); c.allocator.destroy(wait_group); } + errdefer wait_group.wait(); { var total_count: usize = 0; for (chunks, chunk_contexts) |*chunk, *chunk_ctx| { diff --git a/src/thread_pool.zig b/src/thread_pool.zig index 8d5f859d44538c..e7e8b8d1071fe7 100644 --- a/src/thread_pool.zig +++ b/src/thread_pool.zig @@ -134,14 +134,10 @@ pub const Batch = struct { pub const WaitGroup = struct { mutex: std.Thread.Mutex = .{}, counter: u32 = 0, - event: std.Thread.ResetEvent, + event: std.Thread.ResetEvent = .{}, pub fn init(self: *WaitGroup) void { - self.* = .{ - .mutex = .{}, - .counter = 0, - .event = undefined, - }; + self.* = .{}; } pub fn deinit(self: *WaitGroup) void { diff --git a/src/work_pool.zig b/src/work_pool.zig index 5cbad488ff22c2..b9e1bd157315b1 100644 --- a/src/work_pool.zig +++ b/src/work_pool.zig @@ -14,13 +14,23 @@ pub fn NewWorkPool(comptime max_threads: ?usize) type { @setCold(true); pool = ThreadPool.init(.{ - .max_threads = max_threads orelse @max(@as(u32, @truncate(std.Thread.getCpuCount() catch 0)), 2), + .max_threads = max_threads orelse @max(2, max_threads: { + if (bun.getenvZ("GOMAXPROCS")) |max_procs| try_override: { + break :max_threads std.fmt.parseInt(u32, max_procs, 10) catch + break :try_override; + } + + break :max_threads @as(u32, @truncate(std.Thread.getCpuCount() catch 0)); + }), .stack_size = ThreadPool.default_thread_stack_size, }); return &pool; } + + /// Initialization of WorkPool is not thread-safe, as it is + /// assumed a single main thread sets everything up. Calling + /// this afterwards is thread-safe. pub inline fn get() *ThreadPool { - // lil racy if (loaded) return &pool; loaded = true; diff --git a/test/bundler/bun-build-api.test.ts b/test/bundler/bun-build-api.test.ts index 5f5e89634b169a..586361a9bf084b 100644 --- a/test/bundler/bun-build-api.test.ts +++ b/test/bundler/bun-build-api.test.ts @@ -14,6 +14,43 @@ describe("Bun.build", () => { throw new Error("should have thrown"); }); + // https://github.com/oven-sh/bun/issues/12818 + test("sourcemap + build error crash case", async () => { + const dir = tempDirWithFiles("build", { + "/src/file1.ts": ` + import { A } from './dir'; + console.log(A); + `, + "/src/dir/index.ts": ` + import { B } from "./file3"; + export const A = [B] + `, + "/src/dir/file3.ts": ` + import { C } from "../file1"; // error + export const B = C; + `, + "/src/package.json": ` + { "type": "module" } + `, + "/src/tsconfig.json": ` + { + "extends": "../tsconfig.json", + "compilerOptions": { + "target": "ESNext", + "module": "ESNext", + "types": [] + } + } + `, + }); + const y = await Bun.build({ + entrypoints: [join(dir, "src/file1.ts")], + outdir: join(dir, "out"), + sourcemap: "external", + external: ["@minecraft"], + }); + }); + test("invalid options throws", async () => { expect(() => Bun.build({} as any)).toThrow(); expect(() =>