From e2e876bfa16b615a1306294a655761dfd3c227b9 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Mon, 7 Aug 2023 21:13:33 -0700
Subject: [PATCH] termio/exec: make sure that process is dead before killing
 read thread

---
 src/termio/Exec.zig | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig
index 5e6cc2f3d..eed0c5e93 100644
--- a/src/termio/Exec.zig
+++ b/src/termio/Exec.zig
@@ -217,15 +217,16 @@ pub fn threadExit(self: *Exec, data: ThreadData) void {
     // Clear out our data since we're not active anymore.
     self.data = null;
 
-    // Quit our read thread first so that we aren't simultaneously
-    // performing a read/close on the pty fd.
-    _ = std.os.write(data.read_thread_pipe, "x") catch |err|
-        log.warn("error writing to read thread quit pipe err={}", .{err});
-    data.read_thread.join();
-
     // Stop our subprocess
     if (data.ev.process_exited) self.subprocess.externalExit();
     self.subprocess.stop();
+
+    // Quit our read thread after exiting the subprocess so that
+    // we don't get stuck waiting for data to stop flowing if it is
+    // a particularly noisy process.
+    _ = std.os.write(data.read_thread_pipe, "x") catch |err|
+        log.warn("error writing to read thread quit pipe err={}", .{err});
+    data.read_thread.join();
 }
 
 /// Update the configuration.
@@ -671,6 +672,7 @@ const Subprocess = struct {
     /// Clean up the subprocess. This will stop the subprocess if it is started.
     pub fn deinit(self: *Subprocess) void {
         self.stop();
+        if (self.pty) |*pty| pty.deinit();
         self.arena.deinit();
         self.* = undefined;
     }
@@ -779,7 +781,8 @@ const Subprocess = struct {
     }
 
     /// Stop the subprocess. This is safe to call anytime. This will wait
-    /// for the subprocess to end so it will block.
+    /// for the subprocess to end so it will block. This does not close
+    /// the pty.
     pub fn stop(self: *Subprocess) void {
         // Kill our command
         if (self.command) |*cmd| {
@@ -800,14 +803,6 @@ const Subprocess = struct {
                 self.flatpak_command = null;
             }
         }
-
-        // Close our PTY. We do this after killing our command because on
-        // macOS, close will block until all blocking operations read/write
-        // are done with it and our reader thread is probably still alive.
-        if (self.pty) |*pty| {
-            pty.deinit();
-            self.pty = null;
-        }
     }
 
     /// Resize the pty subprocess. This is safe to call anytime.