From 2b32b5b75cfe7a31a6043a82d111065b0279b875 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <m@mitchellh.com>
Date: Wed, 5 Aug 2026 14:01:53 -0700
Subject: [PATCH] core: close retained temp directory handles

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.
---
 src/Surface.zig    |  7 +++-
 src/os/TempDir.zig | 94 ++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 87 insertions(+), 14 deletions(-)

diff --git a/src/Surface.zig b/src/Surface.zig
index d3eeff48d..4a5a44f56 100644
--- a/src/Surface.zig
+++ b/src/Surface.zig
@@ -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
diff --git a/src/os/TempDir.zig b/src/os/TempDir.zig
index f093ac3a7..ca23426e9 100644
--- a/src/os/TempDir.zig
+++ b/src/os/TempDir.zig
@@ -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);
 }