Repository: alacritty/alacritty (language: rust)

Issue:
alacritty msg create-window leaves a unix fifo behind
I like my terminals popping up fast, so I use this script to start them:
```
/home/greg/.cargo/bin/alacritty msg create-window ||
exec /home/greg/.cargo/bin/alacritty -o 'font.normal.family: JetBrainsMono' -o 'draw_bold_text_with_bright_colors: true'
```

Curiously, starting a new window and closing it reliably leaves an extra fd behind.

The way I monitor it is I lookup the `alacritty` pid and then run this loop:
```
% while sleep 1; do lsof -p 17550 | grep -v mem | wc -l; done
31
31
31
... launched then closed a window captured logs of FDs attached
32
38 <- new window
33 <- closed
39 <- new window
34 <- closed
40 <- new window
35 <- closed
35
35
```

The delta between the initial set of FDs and after the second window is closed is this single FIFO.
% diff /tmp/{1,3}.txt
28a29
> alacritty 17550 greg   30u     unix 0x000000003920e72d       0t0  167503 type=STREAM (CONNECTED)

Starting more windows adds more such FIFOs left behind.

### System

OS: Linux 6.1.0-21-amd64 (also OpenBSD)
Version: alacritty 0.13.2
Linux/BSD: X11

### Logs

FDs right after the first process starts [1.txt](https://github.com/alacritty/alacritty/files/15329794/1.txt)
FDs when the second window is opened [2.txt](https://github.com/alacritty/alacritty/files/15329795/2.txt)
FDs after the second window is closed [3.txt](https://github.com/alacritty/alacritty/files/15329796/3.txt)



Discussion / hints:
Very odd, I can confirm this behavior, but even explicitly shutting down the socket streams does not seem to fix it.

This does not fix it:

```diff
diff --git a/alacritty/src/ipc.rs b/alacritty/src/ipc.rs
index 1cb7a1c8..82cbf698 100644
--- a/alacritty/src/ipc.rs
+++ b/alacritty/src/ipc.rs
@@ -41,9 +41,9 @@ pub fn spawn_ipc_socket(options: &Options, event_proxy: EventLoopProxy<Event>) -
         let mut data = String::new();
         for stream in listener.incoming().filter_map(Result::ok) {
             data.clear();
-            let mut stream = BufReader::new(stream);
+            let mut xxx = BufReader::new(&stream);

-            match stream.read_line(&mut data) {
+            match xxx.read_line(&mut data) {
                 Ok(0) | Err(_) => continue,
                 Ok(_) => (),
             };
@@ -72,6 +72,8 @@ pub fn spawn_ipc_socket(options: &Options, event_proxy: EventLoopProxy<Event>) -
                     let _ = event_proxy.send_event(event);
                 },
             }
+
+            stream.shutdown(std::net::Shutdown::Both).unwrap();
         }
     });
```

This does:

```diff
diff --git a/alacritty/src/ipc.rs b/alacritty/src/ipc.rs
index 1cb7a1c8..0d2f7870 100644
--- a/alacritty/src/ipc.rs
+++ b/alacritty/src/ipc.rs
@@ -40,38 +40,6 @@ pub fn spawn_ipc_socket(options: &Options, event_proxy: EventLoopProxy<Event>) -
     thread::spawn_named("socket listener", move || {
         let mut data = String::new();
         for stream in listener.incoming().filter_map(Result::ok) {
-            data.clear();
-            let mut stream = BufReader::new(stream);
-
-            match stream.read_line(&mut data) {
-                Ok(0) | Err(_) => continue,
-                Ok(_) => (),
-            };
-
-            // Read pending events on socket.
-            let message: SocketMessage = match serde_json::from_str(&data) {
-                Ok(message) => message,
-                Err(err) => {
-                    warn!("Failed to convert data from socket: {}", err);
-                    continue;
-                },
-            };
-
-            // Handle IPC events.
-            match message {
-                SocketMessage::CreateWindow(options) => {
-                    let event = Event::new(EventType::CreateWindow(options), None);
-                    let _ = event_proxy.send_event(event);
-                },
-                SocketMessage::Config(ipc_config) => {
-                    let window_id = ipc_config
-                        .window_id
-                        .and_then(|id| u64::try_from(id).ok())
-                        .map(WindowId::from);
-                    let event = Event::new(EventType::IpcConfig(ipc_config), window_id);
-                    let _ = event_proxy.send_event(event);
-                },
-            }
         }
     });
```

So I'd assume the issue is somewhere in-between. Don't have time to look more into it myself but I figure it shouldn't be that hart to narrow down further.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b4947c03a83..0c6aafe3ed6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -24,6 +24,7 @@ Notable changes to the `alacritty_terminal` crate are documented in its
 - Dynamic title disabled for new windows when initial one has title as CLI option
 - While terminal in mouse mode, mouse bindings that used the shift modifier and
   had multiple actions only performed the first action
+- Leaking FDs when closing windows on Unix systems
 
 ## 0.13.2
 
diff --git a/alacritty_terminal/src/tty/unix.rs b/alacritty_terminal/src/tty/unix.rs
index 1336fd04364..1a2104c6e0b 100644
--- a/alacritty_terminal/src/tty/unix.rs
+++ b/alacritty_terminal/src/tty/unix.rs
@@ -19,8 +19,8 @@ use rustix_openpty::openpty;
 use rustix_openpty::rustix::termios::Winsize;
 #[cfg(any(target_os = "linux", target_os = "macos"))]
 use rustix_openpty::rustix::termios::{self, InputModes, OptionalActions};
-use signal_hook::consts as sigconsts;
-use signal_hook::low_level::pipe as signal_pipe;
+use signal_hook::low_level::{pipe as signal_pipe, unregister as unregister_signal};
+use signal_hook::{consts as sigconsts, SigId};
 
 use crate::event::{OnResize, WindowSize};
 use crate::tty::{ChildEvent, EventedPty, EventedReadWrite, Options};
@@ -102,6 +102,7 @@ pub struct Pty {
     child: Child,
     file: File,
     signals: UnixStream,
+    sig_id: SigId,
 }
 
 impl Pty {
@@ -260,13 +261,13 @@ pub fn from_fd(config: &Options, window_id: u64, master: OwnedFd, slave: OwnedFd
     }
 
     // Prepare signal handling before spawning child.
-    let signals = {
+    let (signals, sig_id) = {
         let (sender, recv) = UnixStream::pair()?;
 
         // Register the recv end of the pipe for SIGCHLD.
-        signal_pipe::register(sigconsts::SIGCHLD, sender)?;
+        let sig_id = signal_pipe::register(sigconsts::SIGCHLD, sender)?;
         recv.set_nonblocking(true)?;
-        recv
+        (recv, sig_id)
     };
 
     match builder.spawn() {
@@ -277,7 +278,7 @@ pub fn from_fd(config: &Options, window_id: u64, master: OwnedFd, slave: OwnedFd
                 set_nonblocking(master_fd);
             }
 
-            Ok(Pty { child, file: File::from(master), signals })
+            Ok(Pty { child, file: File::from(master), signals, sig_id })
         },
         Err(err) => Err(Error::new(
             err.kind(),
@@ -296,6 +297,10 @@ impl Drop for Pty {
         unsafe {
             libc::kill(self.child.id() as i32, libc::SIGHUP);
         }
+
+        // Clear signal-hook handler.
+        unregister_signal(self.sig_id);
+
         let _ = self.child.wait();
     }
 }


Test patch: