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

Revamp signal handling. #1480

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

falsifian
Copy link

@falsifian falsifian commented Jan 12, 2024

I ran into a bug related to bspwm's signal handling, and then noticed a couple of other issues, so decided to try redoing it.

The bug: pipes like find / | : would hang when launched from bspwm instead of terminating right away. The reason was that bspwm ignores SIGPIPE, and that status got passed on to anything launched via bspwmrc, through sxhkdrc, xterm, my shell, all the way through to the find process which is supposed to die when its output is cut off.

NB I've only just started testing this change by using it (edit to add: on OpenBSD, so testing on Linux might be valuable). If there are particular things for me to try out (e.g. the reasons the signal handlers were installed in the first place), let me know.

Details:

  • Change from signal to sigaction, which is more portable.

  • Install a handler for SIGPIPE instead of ignoring it, to avoid passing on the ignored status to other programs launched from bspwm.

  • Use SA_NOCLDWAIT to avoid the need to call waitpid. (NB if waitpid were called in the handler, it would be a good idea to protect errno.)

  • Declare the "running" global as volatile sig_atomic_t, as recommended for values modified from signal handlers.

- Change from signal to sigaction, which is more portable.

- Install a handler for SIGPIPE instead of ignoring it, to avoid
  passing on the ignored status to other programs launched from
  bspwm.

- Use SA_NOCLDWAIT to avoid the need to call waitpid. (NB if waitpid
  were called in the handler, it would be a good idea to protect
  errno.)

- Declare the "running" global as volatile sig_atomic_t, as recommended
  for values modified from signal handlers.
@ortango
Copy link

ortango commented Jan 13, 2024

Running linux here and everything seems to be working fine with the patch.
I tried getting some zombied processes in bspwmrc and subscribe seems to work fine.

I needed to add a signal.h include in bspwm.h to build.

fwiw, i could not get the original issue to trigger - bash as shell and dash as sxhkd shell. I tried directly in bspwmrc, but no hang with the find command.

@falsifian
Copy link
Author

Running linux here and everything seems to be working fine with the patch. I tried getting some zombied processes in bspwmrc and subscribe seems to work fine.

I needed to add a signal.h include in bspwm.h to build.

Thanks! Added.

fwiw, i could not get the original issue to trigger - bash as shell and dash as sxhkd shell. I tried directly in bspwmrc, but no hang with the find command.

Interesting. I don't have a Linux computer I can run bspwm on, but I do have Linux shell access. I'm curious what these two commands do for you:

  • perl -e '$SIG{PIPE}="IGNORE";exec "find /|:"'

On OpenBSD and my Linux shell account, the find command keeps running (and on Linux, writes permission errors to stderr).

  • perl -e 'exec "find /|:"'

Baseline to compare to. Exits right away for me.

(Tangent: I tried making Python versions of the above commands in case you have Python more readily available, but as far as I can tell Python's os.system suffers from the same SIGPIPE issue as bspwm. I found this related bug saying at least it got fixed for the subprocess module but I'm too lazy to figure out how to use that instead of os.system.)

@ortango
Copy link

ortango commented Jan 14, 2024

The perl commands act as expected, pipe ignore version with perm errors to stderr and normal version exits immediately.

Not sure why I'm not getting the sig_ign behavior you get.

linux's procfs has bitmasks for the caught/blocked/ignored signals of a process and makes it easy enough to check. Certainly am inheriting the signal handlers from bspwm in bspwmrc and external rules script (sxhkd is not launched from bspwmrc and is not a child of bspwm on my machine)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants