fix segfault during non-install script execution (#2045)

4 files changed, 59 insertions, 181 deletions

diff --git a/src/install/dependency.zig b/src/install/dependency.zig
index 2cc3d4177..973a8b997 100644
--- a/src/install/dependency.zig
+++ b/src/install/dependency.zig
@@ -15,12 +15,6 @@ const string = @import("../string_types.zig").string;
 const strings = @import("../string_immutable.zig");
 const Dependency = @This();
 
-pub const Pair = struct {
-    resolution_id: Install.PackageID = Install.invalid_package_id,
-    dependency: Dependency = .{},
-    failed: ?anyerror = null,
-};
-
 pub const URI = union(Tag) {
     local: String,
     remote: String,
@@ -44,8 +38,8 @@ pub const URI = union(Tag) {
 };
 
 name_hash: PackageNameHash = 0,
-name: String = String{},
-version: Dependency.Version = Dependency.Version{},
+name: String = .{},
+version: Dependency.Version = .{},
 
 /// This is how the dependency is specified in the package.json file.
 /// This allows us to track whether a package originated in any permutation of:
@@ -55,7 +49,7 @@ version: Dependency.Version = Dependency.Version{},
 /// - `peerDependencies`
 /// Technically, having the same package name specified under multiple fields is invalid
 /// But we don't want to allocate extra arrays for them. So we use a bitfield instead.
-behavior: Behavior = Behavior.uninitialized,
+behavior: Behavior = .uninitialized,
 
 /// Sorting order for dependencies is:
 /// 1. [`dependencies`, `devDependencies`, `optionalDependencies`, `peerDependencies`]
diff --git a/src/install/extract_tarball.zig b/src/install/extract_tarball.zig
index 7ba085297..7563655e1 100644
--- a/src/install/extract_tarball.zig
+++ b/src/install/extract_tarball.zig
@@ -24,11 +24,11 @@ cache_dir: std.fs.Dir,
 temp_dir: std.fs.Dir,
 dependency_id: DependencyID,
 skip_verify: bool = false,
-integrity: Integrity = Integrity{},
+integrity: Integrity = .{},
 url: string = "",
 package_manager: *PackageManager,
 
-pub inline fn run(this: ExtractTarball, task_id: u64, bytes: []const u8) !Install.ExtractData {
+pub inline fn run(this: ExtractTarball, bytes: []const u8) !Install.ExtractData {
     if (!this.skip_verify and this.integrity.tag.isSupported()) {
         if (!this.integrity.verify(bytes)) {
             Output.prettyErrorln("<r><red>Integrity check failed<r> for tarball: {s}", .{this.name.slice()});
@@ -36,7 +36,7 @@ pub inline fn run(this: ExtractTarball, task_id: u64, bytes: []const u8) !Instal
             return error.IntegrityCheckFailed;
         }
     }
-    return this.extract(bytes, task_id);
+    return this.extract(bytes);
 }
 
 pub fn buildURL(
@@ -149,7 +149,7 @@ threadlocal var final_path_buf: [bun.MAX_PATH_BYTES]u8 = undefined;
 threadlocal var folder_name_buf: [bun.MAX_PATH_BYTES]u8 = undefined;
 threadlocal var json_path_buf: [bun.MAX_PATH_BYTES]u8 = undefined;
 
-fn extract(this: *const ExtractTarball, tgz_bytes: []const u8, task_id: u64) !Install.ExtractData {
+fn extract(this: *const ExtractTarball, tgz_bytes: []const u8) !Install.ExtractData {
     var tmpdir = this.temp_dir;
     var tmpname_buf: [256]u8 = undefined;
     const name = this.name.slice();
@@ -401,6 +401,5 @@ fn extract(this: *const ExtractTarball, tgz_bytes: []const u8, task_id: u64) !In
         .json_path = ret_json_path,
         .json_buf = json_buf,
         .json_len = json_len,
-        .task_id = task_id,
     };
 }
diff --git a/src/install/install.zig b/src/install/install.zig
index a411d2098..3034b82f8 100644
--- a/src/install/install.zig
+++ b/src/install/install.zig
@@ -567,7 +567,6 @@ const Task = struct {
             },
             .extract => {
                 const result = this.request.extract.tarball.run(
-                    this.id,
                     this.request.extract.network.response_buffer.toOwnedSliceLeaky(),
                 ) catch |err| {
                     if (comptime Environment.isDebug) {
@@ -633,7 +632,6 @@ pub const ExtractData = struct {
     json_path: string = "",
     json_buf: []u8 = "",
     json_len: usize = 0,
-    task_id: u64 = 0,
 };
 
 const PackageInstall = struct {
@@ -1410,19 +1408,15 @@ pub const Resolution = @import("./resolution.zig").Resolution;
 const Progress = std.Progress;
 const TaggedPointer = @import("../tagged_pointer.zig");
 const TaskCallbackContext = union(Tag) {
-    dependency: PackageID,
-    request_id: PackageID,
-    root_dependency: PackageID,
+    dependency: DependencyID,
+    root_dependency: DependencyID,
     root_request_id: PackageID,
-    node_modules_folder: u32, // Really, this is a file descriptor
-    root_node_modules_folder: u32, // Really, this is a file descriptor
+    node_modules_folder: std.os.fd_t,
     pub const Tag = enum {
         dependency,
-        request_id,
         node_modules_folder,
         root_dependency,
         root_request_id,
-        root_node_modules_folder,
     };
 };
 
@@ -1474,11 +1468,6 @@ pub const PackageManager = struct {
     root_package_json_file: std.fs.File,
     root_dependency_list: Lockfile.DependencySlice = .{},
 
-    /// Used to make "dependencies" optional in the main package
-    /// Depended on packages have to explicitly list their dependencies
-    dynamic_root_dependencies: ?std.ArrayList(Dependency.Pair) = null,
-    // remote_dependencies: RemoteDependency.List = .{},
-
     thread_pool: ThreadPool,
 
     manifests: PackageManifestMap = PackageManifestMap{},
@@ -1530,24 +1519,18 @@ pub const PackageManager = struct {
         }
 
         pub inline fn getonDependencyError(t: @This()) *const fn (ctx: *anyopaque, Dependency, DependencyID, anyerror) void {
-            return bun.cast(*const fn (ctx: *anyopaque, Dependency, PackageID, anyerror) void, t.handler);
+            return bun.cast(*const fn (ctx: *anyopaque, Dependency, DependencyID, anyerror) void, t.handler);
         }
     };
 
     pub fn failRootResolution(this: *PackageManager, dependency: *const Dependency, dependency_id: DependencyID, err: anyerror) void {
-        if (this.dynamic_root_dependencies) |*dynamic| {
-            dynamic.items[dependency_id].failed = err;
-            if (this.onWake.context) |ctx| {
-                this.onWake.getonDependencyError()(
-                    ctx,
-                    dependency.*,
-                    dependency_id,
-                    err,
-                );
-            }
-        } else {
-            // this means a bug
-            bun.unreachablePanic("assignRootResolution: dependency_id: {d} out of bounds", .{dependency_id});
+        if (this.onWake.context) |ctx| {
+            this.onWake.getonDependencyError()(
+                ctx,
+                dependency.*,
+                dependency_id,
+                err,
+            );
         }
     }
 
@@ -1572,6 +1555,7 @@ pub const PackageManager = struct {
         not_found: void,
         failure: anyerror,
     };
+
     pub fn enqueueDependencyToRoot(
         this: *PackageManager,
         name: []const u8,
@@ -1580,23 +1564,21 @@ pub const PackageManager = struct {
         behavior: Dependency.Behavior,
         is_main: bool,
     ) DependencyToEnqueue {
-        var root_deps = this.dynamicRootDependencies();
-        const existing: []const Dependency.Pair = root_deps.items;
-        var str_buf = this.lockfile.buffers.string_bytes.items;
-        for (existing) |pair, i| {
-            if (strings.eqlLong(this.lockfile.str(&pair.dependency.name), name, true)) {
-                if (pair.dependency.version.eql(version, str_buf, version_buf)) {
-                    if (pair.resolution_id != invalid_package_id) {
-                        return .{
-                            .resolution = .{
-                                .resolution = this.lockfile.packages.items(.resolution)[pair.resolution_id],
-                                .package_id = pair.resolution_id,
-                            },
-                        };
-                    }
-                    return .{ .pending = @truncate(DependencyID, i) };
-                }
-            }
+        const str_buf = this.lockfile.buffers.string_bytes.items;
+        for (this.lockfile.buffers.dependencies.items) |dependency, dependency_id| {
+            if (!strings.eqlLong(dependency.name.slice(str_buf), name, true)) continue;
+            if (!dependency.version.eql(version, str_buf, version_buf)) continue;
+            return switch (this.lockfile.buffers.resolutions.items[dependency_id]) {
+                invalid_package_id => .{
+                    .pending = @truncate(DependencyID, dependency_id),
+                },
+                else => |resolution_id| .{
+                    .resolution = .{
+                        .resolution = this.lockfile.packages.items(.resolution)[resolution_id],
+                        .package_id = resolution_id,
+                    },
+                },
+            };
         }
 
         var builder = this.lockfile.stringBuilder();
@@ -1612,10 +1594,10 @@ pub const PackageManager = struct {
 
         const cloned_dependency = dependency.cloneWithDifferentBuffers(name, version_buf, @TypeOf(&builder), &builder) catch unreachable;
         builder.clamp();
-        const index = @truncate(DependencyID, root_deps.items.len);
-        root_deps.append(.{
-            .dependency = cloned_dependency,
-        }) catch unreachable;
+        const index = @truncate(DependencyID, this.lockfile.buffers.dependencies.items.len);
+        this.lockfile.buffers.dependencies.append(this.allocator, cloned_dependency) catch unreachable;
+        this.lockfile.buffers.resolutions.append(this.allocator, invalid_package_id) catch unreachable;
+        if (Environment.allow_assert) std.debug.assert(this.lockfile.buffers.dependencies.items.len == this.lockfile.buffers.resolutions.items.len);
         if (is_main) {
             this.enqueueDependencyWithMainAndSuccessFn(
                 index,
@@ -1625,7 +1607,6 @@ pub const PackageManager = struct {
                 assignRootResolution,
                 failRootResolution,
             ) catch |err| {
-                root_deps.items.len = index;
                 return .{ .failure = err };
             };
         } else {
@@ -1637,30 +1618,22 @@ pub const PackageManager = struct {
                 assignRootResolution,
                 failRootResolution,
             ) catch |err| {
-                root_deps.items.len = index;
                 return .{ .failure = err };
             };
         }
 
-        if (root_deps.items[index].failed) |fail| {
-            root_deps.items.len = index;
-            return .{ .failure = fail };
-        }
-
-        const resolution_id = root_deps.items[index].resolution_id;
+        const resolution_id = this.lockfile.buffers.resolutions.items[index];
 
         // check if we managed to synchronously resolve the dependency
-        if (resolution_id != invalid_package_id) {
-            this.drainDependencyList();
-            return .{
-                .resolution = .{
-                    .resolution = this.lockfile.packages.items(.resolution)[resolution_id],
-                    .package_id = resolution_id,
-                },
-            };
-        }
+        if (resolution_id == invalid_package_id) return .{ .pending = index };
 
-        return .{ .pending = index };
+        this.drainDependencyList();
+        return .{
+            .resolution = .{
+                .resolution = this.lockfile.packages.items(.resolution)[resolution_id],
+                .package_id = resolution_id,
+            },
+        };
     }
 
     pub fn globalLinkDir(this: *PackageManager) !std.fs.IterableDir {
@@ -2323,21 +2296,11 @@ pub const PackageManager = struct {
 
     fn assignRootResolution(this: *PackageManager, dependency_id: DependencyID, package_id: PackageID) void {
         if (comptime Environment.allow_assert) {
+            std.debug.assert(dependency_id < this.lockfile.buffers.resolutions.items.len);
             std.debug.assert(package_id < this.lockfile.packages.len);
+            std.debug.assert(this.lockfile.buffers.resolutions.items[dependency_id] == invalid_package_id);
         }
-        if (this.dynamic_root_dependencies) |*dynamic| {
-            if (comptime Environment.allow_assert) {
-                std.debug.assert(dependency_id < dynamic.items.len);
-                std.debug.assert(dynamic.items[dependency_id].resolution_id == invalid_package_id);
-            }
-            dynamic.items[dependency_id].resolution_id = package_id;
-        } else {
-            if (comptime Environment.allow_assert) {
-                std.debug.assert(dependency_id < this.lockfile.buffers.resolutions.items.len);
-                std.debug.assert(this.lockfile.buffers.resolutions.items[dependency_id] == invalid_package_id);
-            }
-            this.lockfile.buffers.resolutions.items[dependency_id] = package_id;
-        }
+        this.lockfile.buffers.resolutions.items[dependency_id] = package_id;
     }
 
     fn getOrPutResolvedPackage(
@@ -2483,23 +2446,6 @@ pub const PackageManager = struct {
         return &task.threadpool_task;
     }
 
-    pub fn dynamicRootDependencies(this: *PackageManager) *std.ArrayList(Dependency.Pair) {
-        if (this.dynamic_root_dependencies == null) {
-            const root_deps = this.lockfile.rootPackage().?.dependencies.get(this.lockfile.buffers.dependencies.items);
-
-            this.dynamic_root_dependencies = std.ArrayList(Dependency.Pair).initCapacity(this.allocator, root_deps.len) catch unreachable;
-            this.dynamic_root_dependencies.?.items.len = root_deps.len;
-            for (root_deps) |dep, i| {
-                this.dynamic_root_dependencies.?.items[i] = .{
-                    .dependency = dep,
-                    .resolution_id = invalid_package_id,
-                };
-            }
-        }
-
-        return &this.dynamic_root_dependencies.?;
-    }
-
     pub fn writeYarnLock(this: *PackageManager) !void {
         var printer = Lockfile.Printer{
             .lockfile = this.lockfile,
@@ -2544,10 +2490,6 @@ pub const PackageManager = struct {
     }
 
     pub fn isRootDependency(this: *const PackageManager, id: DependencyID) bool {
-        if (this.dynamic_root_dependencies != null) {
-            return false;
-        }
-
         return this.root_dependency_list.contains(id);
     }
 
@@ -3007,9 +2949,8 @@ pub const PackageManager = struct {
             },
 
             .root_dependency => |dependency_id| {
-                const pair = this.dynamicRootDependencies().items[dependency_id];
-                const dependency = pair.dependency;
-                const resolution = pair.resolution_id;
+                const dependency = this.lockfile.buffers.dependencies.items[dependency_id];
+                const resolution = this.lockfile.buffers.resolutions.items[dependency_id];
 
                 try this.enqueueDependencyWithMainAndSuccessFn(
                     dependency_id,
@@ -3021,8 +2962,8 @@ pub const PackageManager = struct {
                 );
 
                 if (any_root) |ptr| {
-                    const new_resolution_id = this.dynamicRootDependencies().items[dependency_id].resolution_id;
-                    if (new_resolution_id != pair.resolution_id) {
+                    const new_resolution_id = this.lockfile.buffers.resolutions.items[dependency_id];
+                    if (new_resolution_id != resolution) {
                         ptr.* = true;
                     }
                 }
@@ -3182,15 +3123,6 @@ pub const PackageManager = struct {
                         const err = task.http.err orelse error.HTTPError;
 
                         if (@TypeOf(callbacks.onPackageManifestError) != void) {
-                            if (manager.dynamic_root_dependencies) |*root_deps| {
-                                var deps: []Dependency.Pair = root_deps.items;
-                                for (deps) |*dep| {
-                                    if (strings.eqlLong(manager.lockfile.str(&dep.dependency.name), name.slice(), true)) {
-                                        dep.failed = dep.failed orelse err;
-                                    }
-                                }
-                            }
-
                             callbacks.onPackageManifestError(
                                 extract_ctx,
                                 name.slice(),
@@ -3226,15 +3158,6 @@ pub const PackageManager = struct {
                                 else => error.PackageManifestHTTP5xx,
                             };
 
-                            if (manager.dynamic_root_dependencies) |*root_deps| {
-                                var deps: []Dependency.Pair = root_deps.items;
-                                for (deps) |*dep| {
-                                    if (strings.eql(manager.lockfile.str(&dep.dependency.name), name.slice())) {
-                                        dep.failed = dep.failed orelse err;
-                                    }
-                                }
-                            }
-
                             callbacks.onPackageManifestError(
                                 extract_ctx,
                                 name.slice(),
@@ -3363,13 +3286,6 @@ pub const PackageManager = struct {
                         const package_id = manager.lockfile.buffers.resolutions.items[extract.dependency_id];
 
                         if (@TypeOf(callbacks.onPackageDownloadError) != void) {
-                            if (manager.dynamic_root_dependencies) |*root_deps| {
-                                for (root_deps.items) |*dep| {
-                                    if (dep.resolution_id == package_id) {
-                                        dep.failed = err;
-                                    }
-                                }
-                            }
                             callbacks.onPackageDownloadError(
                                 extract_ctx,
                                 package_id,
@@ -3409,14 +3325,6 @@ pub const PackageManager = struct {
                             };
                             const package_id = manager.lockfile.buffers.resolutions.items[extract.dependency_id];
 
-                            if (manager.dynamic_root_dependencies) |*root_deps| {
-                                for (root_deps.items) |*dep| {
-                                    if (dep.resolution_id == package_id) {
-                                        dep.failed = err;
-                                    }
-                                }
-                            }
-
                             callbacks.onPackageDownloadError(
                                 extract_ctx,
                                 package_id,
@@ -3486,15 +3394,6 @@ pub const PackageManager = struct {
                         const err = task.err orelse error.Failed;
 
                         if (@TypeOf(callbacks.onPackageManifestError) != void) {
-                            if (manager.dynamic_root_dependencies) |*root_deps| {
-                                var deps: []Dependency.Pair = root_deps.items;
-                                for (deps) |*dep| {
-                                    if (strings.eql(manager.lockfile.str(&dep.dependency.name), name.slice())) {
-                                        dep.failed = dep.failed orelse err;
-                                    }
-                                }
-                            }
-
                             callbacks.onPackageManifestError(
                                 extract_ctx,
                                 name.slice(),
@@ -3544,15 +3443,6 @@ pub const PackageManager = struct {
                     if (task.status == .fail) {
                         const err = task.err orelse error.TarballFailedToExtract;
                         if (@TypeOf(callbacks.onPackageDownloadError) != void) {
-                            if (manager.dynamic_root_dependencies) |*root_deps| {
-                                var deps: []Dependency.Pair = root_deps.items;
-                                for (deps) |*dep| {
-                                    if (dep.resolution_id == package_id) {
-                                        dep.failed = dep.failed orelse err;
-                                    }
-                                }
-                            }
-
                             callbacks.onPackageDownloadError(
                                 extract_ctx,
                                 package_id,
@@ -3613,7 +3503,7 @@ pub const PackageManager = struct {
                                     needs_flush = true;
                                 },
                                 .root_dependency => |id| {
-                                    manager.dynamicRootDependencies().items[id].dependency.version.value.github.package_name = name;
+                                    manager.lockfile.buffers.dependencies.items[id].version.value.github.package_name = name;
                                     try manager.processDependencyListItem(dep, &any_root);
                                     needs_flush = true;
                                 },
@@ -5871,8 +5761,7 @@ pub const PackageManager = struct {
                 const prev_node_modules_folder = this.node_modules_folder;
                 defer this.node_modules_folder = prev_node_modules_folder;
                 for (callbacks.items) |cb| {
-                    const node_modules_folder = cb.node_modules_folder;
-                    this.node_modules_folder = .{ .dir = .{ .fd = @intCast(bun.FileDescriptor, node_modules_folder) } };
+                    this.node_modules_folder = .{ .dir = .{ .fd = cb.node_modules_folder } };
                     this.installPackageWithNameAndResolution(dependency_id, package_id, log_level, name, resolution);
                 }
             }
@@ -6086,9 +5975,7 @@ pub const PackageManager = struct {
                                         dependency_id,
                                         package_id,
                                         &resolution.value.github,
-                                        .{
-                                            .node_modules_folder = @intCast(u32, this.node_modules_folder.dir.fd),
-                                        },
+                                        .{ .node_modules_folder = this.node_modules_folder.dir.fd },
                                     );
                                 },
                                 .npm => {
@@ -6099,9 +5986,7 @@ pub const PackageManager = struct {
                                         package_id,
                                         resolution.value.npm.version,
                                         resolution.value.npm.url.slice(buf),
-                                        .{
-                                            .node_modules_folder = @intCast(u32, this.node_modules_folder.dir.fd),
-                                        },
+                                        .{ .node_modules_folder = this.node_modules_folder.dir.fd },
                                     );
                                 },
                                 else => {
diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig
index e96a7be80..cb4326699 100644
--- a/src/install/lockfile.zig
+++ b/src/install/lockfile.zig
@@ -2701,7 +2701,7 @@ pub const Package = extern struct {
         try lockfile.buffers.resolutions.ensureUnusedCapacity(lockfile.allocator, total_dependencies_count);
 
         const total_len = lockfile.buffers.dependencies.items.len + total_dependencies_count;
-        std.debug.assert(lockfile.buffers.dependencies.items.len == lockfile.buffers.resolutions.items.len);
+        if (Environment.allow_assert) std.debug.assert(lockfile.buffers.dependencies.items.len == lockfile.buffers.resolutions.items.len);
         const off = lockfile.buffers.dependencies.items.len;
 
         var package_dependencies = lockfile.buffers.dependencies.items.ptr[off..total_len];