Repository: zellij-org/zellij (language: rust)

Issue:
Problems with repeatedly attaching to session - leaks memory and finally crashes
Just some stress testing with rapid attaching and detaching yielded this possible server crash locations:

```
Error occurred in server:

  × Thread 'server_listener' panicked.
  ├─▶ At .../zellij-utils/src/ipc.rs:213:37
  ╰─▶ called `Result::unwrap()` on an `Err` value: EMFILE
```

My build is at dc922907eb3aed6fd7a1ae6a2e23f03042801b28


Discussion / hints:
More information: It seems `EMFILE` is unix errno 24, which means "Too many open files". Can you try reproducing this in a fresh user session? Maybe we have a problem with not closing file descriptors correctly?
Repro:
- in terminal 1: open zellij session
- in terminal 2: `while true; do timeout 0.1 zellij attach; done`

This also leaks memory :)

Here is a version that counts how many attaches were possible:
`n=0; while true; do timeout --preserve-status 0.1 zellij attach || break; ((n++)); done; echo $`
I get exactly 252 attaches before it crashes.

My build is now 5ad0429a
My gut feeling tells me that there might be a fork without CLOEXEC used (either no OCLOEXEC set or no execv being used leading to file descriptor leaking into the child process), but this does not explain the memory leaks.

Background: Rust libstd defaults to a mutex on process spawn, so other threads in the process should not leak it. See also https://github.com/rust-lang/rust/issues/38227.
This bug is also triggered when using something like `zellij run` or `zellij ls` since that behaves like a temporary client or at least connects to the server processes.
Even worse, this seems to "increment the attach counter" for **all** sessions visible so they will eventually crash.
Demo:
`while true; do zellij ls; done`
`while true; do zellij --session aaaa run --close-on-exit -- echo; done`
Any of these will reliably crash **all** your sessions :)
I ran into this issue in practice. We had a number of users running zellij on a single machine until all instances crashed at some point of time. The logs showed the same `EMFILE` error. 

Later I checked the number of open files, and found that zellij was using about 50k open file descriptors! The majority (85%) of them were Unix socket files for the server-client communication. About half of them were used by the "async-std" thread. 

Any ideas on how to get to the root of this issue? Would it be of any merit to build a server-client toy example that uses a LocalSocketListener in a separate thread?
Just submitted a PR that fixes this issue, see #2322 .

Nice! It fixes the crashes.

I fear we might still have a small memory leak tho:
- zellij, default layout with 1 tab, 1 pane, statusbar and tabbar
- directly after start + detach: 631MB resident set size
- 1000x `zellij ls`: 655MB
- 5000x: 751 MB

This works out to 24kB leaked for each client / attach.
@raphCode - these might not strictly be leaks, I think @tlinford has done some digging into the rust allocator and might have a relevant link handy?
> @raphCode - these might not strictly be leaks, I think @tlinford has done some digging into the rust allocator and might have a relevant link handy?

Here is what I found about allocators in another issue: https://github.com/zellij-org/zellij/issues/1625#issuecomment-1411735893
Btw i pushed the branch with jemalloc as allocator here: https://github.com/zellij-org/zellij/tree/jemalloc
I am aware of the allocator playing a role, but this case looks like a leak to me:
Memory rises linearly with attach count, no anomalies. I performed 5000 attach repetitions. Or 5 repetitions of the '1000x attach' case, if you want.

Tell me if any of these would be helpful:
- rechecking with jemalloc branch
- running it under valgrind
- letting the test run longer

Fix patch:
diff --git a/zellij-server/src/lib.rs b/zellij-server/src/lib.rs
index 34c807f16e..6109a3b850 100644
--- a/zellij-server/src/lib.rs
+++ b/zellij-server/src/lib.rs
@@ -20,7 +20,7 @@ use pty_writer::{pty_writer_main, PtyWriteInstruction};
 use std::collections::{HashMap, HashSet};
 use std::{
     path::PathBuf,
-    sync::{Arc, Mutex, RwLock},
+    sync::{Arc, RwLock},
     thread,
 };
 use zellij_utils::envs;
@@ -260,8 +260,6 @@ pub fn start_server(mut os_input: Box<dyn ServerOsApi>, socket_path: PathBuf) {
         })
     });
 
-    let thread_handles = Arc::new(Mutex::new(Vec::new()));
-
     let _ = thread::Builder::new()
         .name("server_listener".to_string())
         .spawn({
@@ -274,7 +272,6 @@ pub fn start_server(mut os_input: Box<dyn ServerOsApi>, socket_path: PathBuf) {
             let session_state = session_state.clone();
             let to_server = to_server.clone();
             let socket_path = socket_path.clone();
-            let thread_handles = thread_handles.clone();
             move || {
                 drop(std::fs::remove_file(&socket_path));
                 let listener = LocalSocketListener::bind(&*socket_path).unwrap();
@@ -288,22 +285,20 @@ pub fn start_server(mut os_input: Box<dyn ServerOsApi>, socket_path: PathBuf) {
                             let session_data = session_data.clone();
                             let session_state = session_state.clone();
                             let to_server = to_server.clone();
-                            thread_handles.lock().unwrap().push(
-                                thread::Builder::new()
-                                    .name("server_router".to_string())
-                                    .spawn(move || {
-                                        route_thread_main(
-                                            session_data,
-                                            session_state,
-                                            os_input,
-                                            to_server,
-                                            receiver,
-                                            client_id,
-                                        )
-                                        .fatal()
-                                    })
-                                    .unwrap(),
-                            );
+                            thread::Builder::new()
+                                .name("server_router".to_string())
+                                .spawn(move || {
+                                    route_thread_main(
+                                        session_data,
+                                        session_state,
+                                        os_input,
+                                        to_server,
+                                        receiver,
+                                        client_id,
+                                    )
+                                    .fatal()
+                                })
+                                .unwrap();
                         },
                         Err(err) => {
                             panic!("err {:?}", err);
@@ -639,11 +634,6 @@ pub fn start_server(mut os_input: Box<dyn ServerOsApi>, socket_path: PathBuf) {
     // Drop cached session data before exit.
     *session_data.write().unwrap() = None;
 
-    thread_handles
-        .lock()
-        .unwrap()
-        .drain(..)
-        .for_each(|h| drop(h.join()));
     drop(std::fs::remove_file(&socket_path));
 }
 


Test patch:
diff --git a/zellij-server/src/tab/unit/tab_integration_tests.rs b/zellij-server/src/tab/unit/tab_integration_tests.rs
index 8ccce263e8..e2e21736a7 100644
--- a/zellij-server/src/tab/unit/tab_integration_tests.rs
+++ b/zellij-server/src/tab/unit/tab_integration_tests.rs
@@ -2,7 +2,7 @@ use super::{Output, Tab};
 use crate::panes::sixel::SixelImageStore;
 use crate::screen::CopyOptions;
 use crate::Arc;
-use crate::Mutex;
+
 use crate::{
     os_input_output::{AsyncReader, Pid, ServerOsApi},
     panes::PaneId,
@@ -11,6 +11,8 @@ use crate::{
     ClientId,
 };
 use std::path::PathBuf;
+use std::sync::Mutex;
+
 use zellij_utils::channels::Receiver;
 use zellij_utils::data::Direction;
 use zellij_utils::data::Resize;