mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-08-24 16:11:43 +00:00
fix screen action fd leak from tempdir (#13644)
Fixes #13219 Screen file actions intentionally retain their temporary directory so the generated path remains valid after dispatch. The directory and parent handles were retained with it, while TempDir.deinit also left the parent handle open. Each successful action leaked two descriptors. Repeated screen, scrollback, or selection writes could exhaust the process descriptor limit and prevent new PTYs, tabs, and windows from opening. Give TempDir an exhaustive close mode that either deletes or retains its contents while always releasing both handles. Defer screen-file cleanup, retaining only after successful dispatch, and cover both lifecycle paths with descriptor tests.
This commit is contained in:
@@ -5694,7 +5694,8 @@ fn writeScreenFile(
|
||||
) !void {
|
||||
// Create a temporary directory to store our scrollback.
|
||||
var tmp_dir = try internal_os.TempDir.init();
|
||||
errdefer tmp_dir.deinit();
|
||||
var retain_tmp_dir = false;
|
||||
defer if (retain_tmp_dir) tmp_dir.close(.retain) else tmp_dir.deinit();
|
||||
|
||||
var filename_buf: [std.fs.max_path_bytes]u8 = undefined;
|
||||
const filename = try std.fmt.bufPrint(
|
||||
@@ -5764,7 +5765,6 @@ fn writeScreenFile(
|
||||
|
||||
const sel = sel_ orelse {
|
||||
// If we have no selection we have no data so we do nothing.
|
||||
tmp_dir.deinit();
|
||||
return;
|
||||
};
|
||||
|
||||
@@ -5818,6 +5818,9 @@ fn writeScreenFile(
|
||||
path,
|
||||
), .unlocked),
|
||||
}
|
||||
|
||||
// The action accepted the path, so retain the file for its consumer.
|
||||
retain_tmp_dir = true;
|
||||
}
|
||||
|
||||
/// Call this to complete a clipboard request sent to apprt. This should
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
//! store temporary data and is destroyed on deinit.
|
||||
const TempDir = @This();
|
||||
|
||||
const builtin = @import("builtin");
|
||||
const std = @import("std");
|
||||
const Dir = std.Io.Dir;
|
||||
const file = @import("file.zig");
|
||||
@@ -61,25 +62,94 @@ pub fn name(self: *TempDir) []const u8 {
|
||||
/// Finish with the temporary directory. This deletes all contents in the
|
||||
/// directory.
|
||||
pub fn deinit(self: *TempDir) void {
|
||||
self.close(.delete);
|
||||
}
|
||||
|
||||
pub const CloseMode = enum { delete, retain };
|
||||
|
||||
/// Close the directory handles, optionally retaining the temporary directory
|
||||
/// and its contents on disk.
|
||||
pub fn close(self: *TempDir, mode: CloseMode) void {
|
||||
self.dir.close(global.io());
|
||||
self.parent.deleteTree(global.io(), self.name()) catch |err|
|
||||
log.err("error deleting temp dir err={}", .{err});
|
||||
switch (mode) {
|
||||
.delete => self.parent.deleteTree(global.io(), self.name()) catch |err|
|
||||
log.err("error deleting temp dir err={}", .{err}),
|
||||
.retain => {},
|
||||
}
|
||||
self.parent.close(global.io());
|
||||
}
|
||||
|
||||
test {
|
||||
const testing = std.testing;
|
||||
|
||||
var td = try init();
|
||||
errdefer td.deinit();
|
||||
var path_buf: [std.fs.max_path_bytes]u8 = undefined;
|
||||
var path_len: usize = undefined;
|
||||
var dir_handle: Dir.Handle = undefined;
|
||||
var parent_handle: Dir.Handle = undefined;
|
||||
{
|
||||
var td = try init();
|
||||
errdefer td.deinit();
|
||||
|
||||
const nameval = td.name();
|
||||
try testing.expect(nameval.len > 0);
|
||||
const nameval = td.name();
|
||||
try testing.expect(nameval.len > 0);
|
||||
|
||||
// Can open a new handle to it proves it exists.
|
||||
var dir = try td.parent.openDir(testing.io, nameval, .{});
|
||||
dir.close(testing.io);
|
||||
// Can open a new handle to it proves it exists.
|
||||
var dir = try td.parent.openDir(testing.io, nameval, .{});
|
||||
dir.close(testing.io);
|
||||
|
||||
// Should be deleted after we deinit
|
||||
td.deinit();
|
||||
try testing.expectError(error.FileNotFound, td.parent.openDir(testing.io, nameval, .{}));
|
||||
path_len = try td.dir.realPath(testing.io, &path_buf);
|
||||
dir_handle = td.dir.handle;
|
||||
parent_handle = td.parent.handle;
|
||||
|
||||
// Should be deleted after we deinit.
|
||||
td.deinit();
|
||||
}
|
||||
|
||||
switch (builtin.os.tag) {
|
||||
.freebsd, .ios, .linux, .macos => {
|
||||
for ([_]Dir.Handle{ dir_handle, parent_handle }) |handle| {
|
||||
const result = std.posix.system.fcntl(handle, std.posix.F.GETFD);
|
||||
try testing.expectEqual(std.posix.E.BADF, std.posix.errno(result));
|
||||
}
|
||||
},
|
||||
|
||||
else => {},
|
||||
}
|
||||
try testing.expectError(
|
||||
error.FileNotFound,
|
||||
Dir.openDirAbsolute(testing.io, path_buf[0..path_len], .{}),
|
||||
);
|
||||
}
|
||||
|
||||
test "close retains temporary directory" {
|
||||
const testing = std.testing;
|
||||
|
||||
var path_buf: [std.fs.max_path_bytes]u8 = undefined;
|
||||
var path_len: usize = undefined;
|
||||
var dir_handle: Dir.Handle = undefined;
|
||||
var parent_handle: Dir.Handle = undefined;
|
||||
{
|
||||
var td = try init();
|
||||
errdefer td.deinit();
|
||||
|
||||
path_len = try td.dir.realPath(testing.io, &path_buf);
|
||||
dir_handle = td.dir.handle;
|
||||
parent_handle = td.parent.handle;
|
||||
|
||||
td.close(.retain);
|
||||
}
|
||||
defer Dir.deleteDirAbsolute(testing.io, path_buf[0..path_len]) catch {};
|
||||
|
||||
switch (builtin.os.tag) {
|
||||
.freebsd, .ios, .linux, .macos => {
|
||||
for ([_]Dir.Handle{ dir_handle, parent_handle }) |handle| {
|
||||
const result = std.posix.system.fcntl(handle, std.posix.F.GETFD);
|
||||
try testing.expectEqual(std.posix.E.BADF, std.posix.errno(result));
|
||||
}
|
||||
},
|
||||
|
||||
else => {},
|
||||
}
|
||||
var dir = try Dir.openDirAbsolute(testing.io, path_buf[0..path_len], .{});
|
||||
dir.close(testing.io);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user