From 9ea0aa49348ca653f7236636e6370abdd1b4767c Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Wed, 1 Jan 2025 14:31:15 -0600 Subject: [PATCH 1/2] core: if we change RLIMIT_NOFILE, reset it when executing commands --- src/Command.zig | 5 +++++ src/global.zig | 9 ++++++++- src/os/file.zig | 24 +++++++++++++++++------- src/os/main.zig | 2 ++ 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/Command.zig b/src/Command.zig index 82b48fa18..2801def36 100644 --- a/src/Command.zig +++ b/src/Command.zig @@ -18,6 +18,7 @@ const Command = @This(); const std = @import("std"); const builtin = @import("builtin"); +const global_state = &@import("global.zig").state; const internal_os = @import("os/main.zig"); const windows = internal_os.windows; const TempDir = internal_os.TempDir; @@ -178,6 +179,10 @@ fn startPosix(self: *Command, arena: Allocator) !void { // If the user requested a pre exec callback, call it now. if (self.pre_exec) |f| f(self); + if (global_state.rlimits.nofile) |lim| { + internal_os.restoreMaxFiles(lim); + } + // Finally, replace our process. _ = posix.execveZ(pathZ, argsZ, envp) catch null; diff --git a/src/global.zig b/src/global.zig index 7e43a9184..b3f35fde5 100644 --- a/src/global.zig +++ b/src/global.zig @@ -27,6 +27,12 @@ pub const GlobalState = struct { alloc: std.mem.Allocator, action: ?cli.Action, logging: Logging, + /// If we change any resource limits for our own purposes, we save the + /// old limits so that they can be restored before we execute any child + /// processes. + rlimits: struct { + nofile: ?internal_os.rlimit = null, + } = .{}, /// The app resources directory, equivalent to zig-out/share when we build /// from source. This is null if we can't detect it. @@ -56,6 +62,7 @@ pub const GlobalState = struct { .alloc = undefined, .action = null, .logging = .{ .stderr = {} }, + .rlimits = .{}, .resources_dir = null, }; errdefer self.deinit(); @@ -124,7 +131,7 @@ pub const GlobalState = struct { std.log.info("libxev backend={}", .{xev.backend}); // First things first, we fix our file descriptors - internal_os.fixMaxFiles(); + self.rlimits.nofile = internal_os.fixMaxFiles(); // Initialize our crash reporting. crash.init(self.alloc) catch |err| { diff --git a/src/os/file.zig b/src/os/file.zig index e0ec2f52c..d89c05e3e 100644 --- a/src/os/file.zig +++ b/src/os/file.zig @@ -4,23 +4,27 @@ const posix = std.posix; const log = std.log.scoped(.os); +pub const rlimit = if (@hasDecl(posix.system, "rlimit")) posix.rlimit else struct {}; + /// This maximizes the number of file descriptors we can have open. We /// need to do this because each window consumes at least a handful of fds. /// This is extracted from the Zig compiler source code. -pub fn fixMaxFiles() void { - if (!@hasDecl(posix.system, "rlimit")) return; +pub fn fixMaxFiles() ?rlimit { + if (!@hasDecl(posix.system, "rlimit")) return null; - var lim = posix.getrlimit(.NOFILE) catch { + const old = posix.getrlimit(.NOFILE) catch { log.warn("failed to query file handle limit, may limit max windows", .{}); - return; // Oh well; we tried. + return null; // Oh well; we tried. }; // If we're already at the max, we're done. - if (lim.cur >= lim.max) { - log.debug("file handle limit already maximized value={}", .{lim.cur}); - return; + if (old.cur >= old.max) { + log.debug("file handle limit already maximized value={}", .{old.cur}); + return old; } + var lim = old; + // Do a binary search for the limit. var min: posix.rlim_t = lim.cur; var max: posix.rlim_t = 1 << 20; @@ -41,6 +45,12 @@ pub fn fixMaxFiles() void { } log.debug("file handle limit raised value={}", .{lim.cur}); + return old; +} + +pub fn restoreMaxFiles(lim: rlimit) void { + if (!@hasDecl(posix.system, "rlimit")) return; + posix.setrlimit(.NOFILE, lim) catch {}; } /// Return the recommended path for temporary files. diff --git a/src/os/main.zig b/src/os/main.zig index 98e57b4fc..af885aa5c 100644 --- a/src/os/main.zig +++ b/src/os/main.zig @@ -32,7 +32,9 @@ pub const getenv = env.getenv; pub const setenv = env.setenv; pub const unsetenv = env.unsetenv; pub const launchedFromDesktop = desktop.launchedFromDesktop; +pub const rlimit = file.rlimit; pub const fixMaxFiles = file.fixMaxFiles; +pub const restoreMaxFiles = file.restoreMaxFiles; pub const allocTmpDir = file.allocTmpDir; pub const freeTmpDir = file.freeTmpDir; pub const isFlatpak = flatpak.isFlatpak; From 8e47d0267bc468483d3fb53acf3aa07fa1e87dea Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Jan 2025 15:04:49 -0800 Subject: [PATCH 2/2] Move resource limits to a dedicated struct, restore before preexec --- src/Command.zig | 8 ++++---- src/global.zig | 29 +++++++++++++++++++++-------- src/os/file.zig | 3 +-- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/Command.zig b/src/Command.zig index 2801def36..6e30eae13 100644 --- a/src/Command.zig +++ b/src/Command.zig @@ -176,13 +176,13 @@ fn startPosix(self: *Command, arena: Allocator) !void { // We don't log because that'll show up in the output. }; + // Restore any rlimits that were set by Ghostty. This might fail but + // any failures are ignored (its best effort). + global_state.rlimits.restore(); + // If the user requested a pre exec callback, call it now. if (self.pre_exec) |f| f(self); - if (global_state.rlimits.nofile) |lim| { - internal_os.restoreMaxFiles(lim); - } - // Finally, replace our process. _ = posix.execveZ(pathZ, argsZ, envp) catch null; diff --git a/src/global.zig b/src/global.zig index b3f35fde5..c00ce27a4 100644 --- a/src/global.zig +++ b/src/global.zig @@ -27,12 +27,7 @@ pub const GlobalState = struct { alloc: std.mem.Allocator, action: ?cli.Action, logging: Logging, - /// If we change any resource limits for our own purposes, we save the - /// old limits so that they can be restored before we execute any child - /// processes. - rlimits: struct { - nofile: ?internal_os.rlimit = null, - } = .{}, + rlimits: ResourceLimits = .{}, /// The app resources directory, equivalent to zig-out/share when we build /// from source. This is null if we can't detect it. @@ -130,8 +125,8 @@ pub const GlobalState = struct { std.log.info("renderer={}", .{renderer.Renderer}); std.log.info("libxev backend={}", .{xev.backend}); - // First things first, we fix our file descriptors - self.rlimits.nofile = internal_os.fixMaxFiles(); + // As early as possible, initialize our resource limits. + self.rlimits = ResourceLimits.init(); // Initialize our crash reporting. crash.init(self.alloc) catch |err| { @@ -181,3 +176,21 @@ pub const GlobalState = struct { } } }; + +/// Maintains the Unix resource limits that we set for our process. This +/// can be used to restore the limits to their original values. +pub const ResourceLimits = struct { + nofile: ?internal_os.rlimit = null, + + pub fn init() ResourceLimits { + return .{ + // Maximize the number of file descriptors we can have open + // because we can consume a lot of them if we make many terminals. + .nofile = internal_os.fixMaxFiles(), + }; + } + + pub fn restore(self: *const ResourceLimits) void { + if (self.nofile) |lim| internal_os.restoreMaxFiles(lim); + } +}; diff --git a/src/os/file.zig b/src/os/file.zig index d89c05e3e..875dd2c25 100644 --- a/src/os/file.zig +++ b/src/os/file.zig @@ -23,9 +23,8 @@ pub fn fixMaxFiles() ?rlimit { return old; } - var lim = old; - // Do a binary search for the limit. + var lim = old; var min: posix.rlim_t = lim.cur; var max: posix.rlim_t = 1 << 20; // But if there's a defined upper bound, don't search, just set it.