-
-
Notifications
You must be signed in to change notification settings - Fork 685
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
Zombie Process on Tab/Pane Close #1286
Comments
I bet this is related to #1029 where whole zellij client processes stay running... |
Hey, just FYI: I'm not reproducing this. Could you tell us a bit more about your OS? Can you reproduce this with other non-vim panes? Does it happen all the time? |
I can reproduce on v0.26.1 and v0.27.0, it happens with all processes started in any pane. If the zellij session is closed, all child processes eventually exit. I even tried |
Ok, I was wrong. This actually seems to be an older regression. bisect log
first bad commit: a14a2f6 |
Hey @raphCode - this was very helpful, thanks! |
With the Here is the diff so you can check if I did it correctly:diff --git a/zellij-server/src/os_input_output.rs b/zellij-server/src/os_input_output.rs
index f0b49c68..7659a5fd 100644
--- a/zellij-server/src/os_input_output.rs
+++ b/zellij-server/src/os_input_output.rs
@@ -20,6 +20,7 @@ use sysinfo::{ProcessExt, ProcessRefreshKind, System, SystemExt};
use nix::pty::{openpty, OpenptyResult, Winsize};
use nix::sys::signal::{kill, Signal};
use nix::sys::termios;
+use nix::sys::wait::waitpid;
use nix::unistd;
use signal_hook::consts::*;
@@ -301,6 +302,7 @@ impl ServerOsApi for ServerOsInputOutput {
}
fn kill(&self, pid: Pid) -> Result<(), nix::Error> {
let _ = kill(pid, Some(Signal::SIGTERM));
+ waitpid(pid, None).unwrap();
Ok(())
}
fn force_kill(&self, pid: Pid) -> Result<(), nix::Error> { Turns out, bash seems to ignore SIGTERM entirely. This is indicated in the manpage:
I think sending SIGHUP would be a better choice, first, because we literally "hang up" the pty connection which is exactly the intention behind this signal.
Also, with I believe the regression came from removing the I am not even sure if this function is called anywhere in the new codebase? What would be its use anyway?
You are using the fish shell which does not catch SIGTERM and exits upon receiving that. |
SIGHUP correctly states the intention behind sending a signal when a pane is closed: The controlling terminal is "hung up". Also, SIGHUP is better suited than SIGTERM since bash ignores the latter. This led to the zombie processes observed by some users. Furthermore, SIGHUP has a special meaning in bash's job control, namely re-sending the signal to all owned jobs before exiting.
Opened a PR to change to SIGHUP |
My PR should have solved the problem, so we can close this. |
I'm seeing zombie processes on tab/pane close. Found out about it through vim open buffers on reopening a previously closed tab that had the same file open. (duplicate/closed issue #518)
Basics
zellij: 0.26.1
os/arch: Darwin/arm64 (M1)
alacritty: 0.11.0-dev
shell: zsh 5.8
Repro Steps:
The text was updated successfully, but these errors were encountered: