Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alacritty msg create-window leaves a unix fifo behind #7983

Closed
blackgnezdo opened this issue May 16, 2024 · 2 comments · Fixed by #7991
Closed

alacritty msg create-window leaves a unix fifo behind #7983

blackgnezdo opened this issue May 16, 2024 · 2 comments · Fixed by #7991

Comments

@blackgnezdo
Copy link

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
FDs when the second window is opened 2.txt
FDs after the second window is closed 3.txt

@chrisduerr
Copy link
Member

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 --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 --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.

@chrisduerr chrisduerr added this to the Version 0.14.0 milestone May 16, 2024
chrisduerr added a commit to chrisduerr/alacritty that referenced this issue May 22, 2024
This patch fixes an issue with signal handling where Alacritty would
permanently create one signal handling FD for each alacritty window
created by an instance. This FD was never released, causing a leak of
the FD.

Closes alacritty#7983.
chrisduerr added a commit to chrisduerr/alacritty that referenced this issue May 22, 2024
This patch fixes an issue with signal handling where Alacritty would
permanently create one signal handling FD for each alacritty window
created by an instance. This FD was never released, causing a leak of
the FD.

Closes alacritty#7983.
@chrisduerr
Copy link
Member

#7991 fixes the issue I was able to reproduce. Please let me know if you can still see any leaks after this patch.

chrisduerr added a commit that referenced this issue May 22, 2024
This patch fixes an issue with signal handling where Alacritty would
permanently create one signal handling FD for each alacritty window
created by an instance. This FD was never released, causing a leak of
the FD.

Closes #7983.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants