gardesk/gar / 0d07d9e

Browse files

Fix i3 IPC FD leak with timeout-based stale client cleanup

Clients that connect but never send messages or subscribe to events
would accumulate indefinitely, eventually exhausting file descriptors.

Add tracking for client creation time and last activity timestamp.
Clients are now cleaned up if they have been idle for 60+ seconds
without subscribing to any events or sending any messages.

This approach is safer than treating UnexpectedEof as disconnect,
which caused issues with polybar's connection handling.
Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
0d07d9e1f5cbe2f4ed340fa2779199322463179e
Parents
d122620
Tree
ffa3995

1 changed file

StatusFile+-
M gar/src/ipc/i3_server.rs 42 7
gar/src/ipc/i3_server.rsmodified
@@ -12,6 +12,7 @@ use std::collections::HashSet;
1212
 use std::io::{BufReader, BufWriter};
1313
 use std::os::unix::net::{UnixListener, UnixStream};
1414
 use std::path::PathBuf;
15
+use std::time::{Duration, Instant};
1516
 
1617
 use super::i3_compat::{read_message, write_event, write_response, EventType, I3Message};
1718
 
@@ -22,12 +23,19 @@ enum ReadResult {
2223
     Disconnected,
2324
 }
2425
 
26
+/// Timeout for clients that have never sent a message and have no subscriptions.
27
+const IDLE_CLIENT_TIMEOUT: Duration = Duration::from_secs(60);
28
+
2529
 /// A connected i3 IPC client.
2630
 struct I3Client {
2731
     stream: UnixStream,
2832
     reader: BufReader<UnixStream>,
2933
     writer: BufWriter<UnixStream>,
3034
     subscriptions: HashSet<String>,
35
+    /// When this client connected.
36
+    created_at: Instant,
37
+    /// When we last received a message from this client.
38
+    last_activity: Option<Instant>,
3139
 }
3240
 
3341
 impl I3Client {
@@ -40,12 +48,33 @@ impl I3Client {
4048
             reader,
4149
             writer,
4250
             subscriptions: HashSet::new(),
51
+            created_at: Instant::now(),
52
+            last_activity: None,
4353
         })
4454
     }
4555
 
56
+    /// Check if this client is stale and should be cleaned up.
57
+    /// A client is stale if it has never sent a message, has no subscriptions,
58
+    /// and has been connected for longer than IDLE_CLIENT_TIMEOUT.
59
+    fn is_stale(&self) -> bool {
60
+        // Clients with subscriptions are waiting for events - keep them
61
+        if !self.subscriptions.is_empty() {
62
+            return false;
63
+        }
64
+        // Clients that have sent messages are active - keep them
65
+        if self.last_activity.is_some() {
66
+            return false;
67
+        }
68
+        // New clients that haven't done anything yet - check timeout
69
+        self.created_at.elapsed() > IDLE_CLIENT_TIMEOUT
70
+    }
71
+
4672
     fn read_message(&mut self) -> ReadResult {
4773
         match read_message(&mut self.reader) {
48
-            Ok(Some(msg)) => ReadResult::Message(msg),
74
+            Ok(Some(msg)) => {
75
+                self.last_activity = Some(Instant::now());
76
+                ReadResult::Message(msg)
77
+            }
4978
             Ok(None) => ReadResult::WouldBlock,
5079
             Err(e) => {
5180
                 tracing::debug!("i3 IPC client read error: {}", e);
@@ -147,21 +176,27 @@ impl I3IpcServer {
147176
 
148177
     /// Process incoming requests from all clients.
149178
     /// Returns a list of (client_index, message) pairs.
179
+    /// Also cleans up stale/disconnected clients.
150180
     pub fn poll_requests(&mut self) -> Vec<(usize, I3Message)> {
151181
         let mut requests = Vec::new();
152
-        let mut disconnected = Vec::new();
182
+        let mut to_remove = Vec::new();
153183
 
154184
         for (i, client) in self.clients.iter_mut().enumerate() {
155185
             match client.read_message() {
156186
                 ReadResult::Message(msg) => requests.push((i, msg)),
157
-                ReadResult::Disconnected => disconnected.push(i),
158
-                ReadResult::WouldBlock => {}
187
+                ReadResult::Disconnected => to_remove.push(i),
188
+                ReadResult::WouldBlock => {
189
+                    // Check if this client is stale (never sent anything, no subscriptions)
190
+                    if client.is_stale() {
191
+                        tracing::debug!("Cleaning up stale i3 IPC client (no activity for {:?})", IDLE_CLIENT_TIMEOUT);
192
+                        to_remove.push(i);
193
+                    }
194
+                }
159195
             }
160196
         }
161197
 
162
-        // Remove disconnected clients (in reverse order to preserve indices)
163
-        for i in disconnected.into_iter().rev() {
164
-            tracing::debug!("i3 IPC client disconnected");
198
+        // Remove disconnected/stale clients (in reverse order to preserve indices)
199
+        for i in to_remove.into_iter().rev() {
165200
             self.clients.remove(i);
166201
         }
167202