nwg-piotr / nwg-drawer

Application drawer for wlroots-based Wayland compositors
MIT License
240 stars 25 forks source link

Launched applications are not detached from the launcher process when resident #39

Closed nightly-brew closed 2 years ago

nightly-brew commented 2 years ago

As per the title, applications are not detached from the launcher process when the drawer instance is resident.

screenshot_2022-01-04_20-22-08_311654307

Normally this goes unnoticed, but if SIGKILL is sent to the launcher, all child processes are terminated as well. Probably related, in the screenshot it's also possible to notice gimp as a zombie process; this happened after quitting gimp.

It would be better instead if processes were left to the init system to handle, similarly as it happens when a standalone instance of the drawer executes an app, then quits.

nightly-brew commented 2 years ago

Took a look at golang docs and at the launch function of nwg-drawer.

The fix might be really easy, actually: the cmd object provided by os/exec contains the lower level Process object as per golang docs. A Process object should be detached upon calling Release on it, so cmd.Process.Release() should do the trick.

Don't take my word for granted though, I'm just at the beginning with golang.

nwg-piotr commented 2 years ago

I'm just at the beginning with golang

LOL, so am I. Thanks for looking into it. Possibly I'll be able to push forward this, and some more pending tasks in next few days.

nightly-brew commented 2 years ago

No problem!

Anyway, I managed to make it work, but I needed to include "sys" and add the setsid attribute to the child process. cmd.Process.Release() didn't seem to impact the spawned program in any way.

cmd.SysProcAttr = &syscall.SysProcAttr {
    Setsid: true,
}

cmd.Start()

This way, the child process isn't killed when the launcher is killed with SIGKILL. As far as I know, this shouldn't prevent the program to be used on BSD if it previously worked, but I can't test right now.

I'm still searching a solution for the zombie processes.

edit: updated my fork, see https://github.com/nightly-brew/nwg-drawer/commit/b78092c935f9669f36e4fe3799d31170691ad3a8.

nwg-piotr commented 2 years ago

edit: updated my fork, see nightly-brew@b78092c

This could be applied, but to be honest I can't reproduce the issue on my machine. If I start some apps from the resident drawer instance, and then kill the drawer, the apps are still alive. As expected when you use cmd.Start().

nightly-brew commented 2 years ago

Interesting. You are right, but I still manage to reproduce the issue. After testing some more though the behaviour seems to be weirdier than I was expecting and I'm starting to doubt if this is an issue with my setup, my knowledge of the system or something else. Would you mind if I explained what I did?

I'm running archlinux on an x86_64 computer. I tested this with keepassxc, gnome clocks and gnome calendar, all of them at the latest release. To launch the resident instance I opened a kitty-terminal window, with bash as a shell, and promptly typed "nwg-drawer -r".

I opened another kitty window on the right with htop, to track the process tree.

What baffles me is the fact that ctrl+c is the same as sending SIGINT. Am I missing something?

nwg-piotr commented 2 years ago

I launch the resident instance of the drawer from the sway config, like exec nwg-drawer -r. Possibly this makes the difference.

nightly-brew commented 2 years ago

So it might be because of the terminal, or it might be caused by bash. I'm going to investigate it tomorrow.

nightly-brew commented 2 years ago

Found out what was happening.

Shells like bash, sh and so on handle started processes as jobs. Each one of the jobs can start other sub-processes to carry on their work. The caller process and its spawns are put in what is called a process group. The process group is identified by the group's leader.

When pressing ctrl+c in bash, a SIGINT is sent to the entire process group of the foreground job, not only to its leader. The difference is that htop and kill do not normally send the signal to the process group, but to the specified pid.

Well... I was sending SIGINT only to nwg-drawer, not to the spawned processes. I verified that the behaviour becomes consistent if I use kill with a 'minus' in front of nwg-drawer's pid, as it sends the signal to the entire group.

What my fix did was effectively separate the spawned processes from the drawer's group process, preventing SIGINT and any other signal intended for the drawer's process group from reaching them.

nwg-piotr commented 2 years ago

What my fix did was (...)

If you think it should be added, feel free to PR.

nightly-brew commented 2 years ago

I hope I'm not introducing any hassle with this PR, it would be a bummer.

nightly-brew commented 2 years ago

Thanks!