From b77e811ad4be2c9c06d2ccfc9848f25e075482ac Mon Sep 17 00:00:00 2001 From: Colin Date: Sun, 21 Apr 2024 11:09:23 +0000 Subject: [PATCH] blast-to-default: leverage sane-die-with-parent --- .../programs/blast-ugjka/blast-to-default | 53 ------------------- hosts/common/programs/blast-ugjka/default.nix | 2 +- hosts/common/programs/mpv/sane_cast/main.lua | 7 ++- .../sane-die-with-parent/sane-die-with-parent | 39 ++++++++++---- 4 files changed, 36 insertions(+), 65 deletions(-) diff --git a/hosts/common/programs/blast-ugjka/blast-to-default b/hosts/common/programs/blast-ugjka/blast-to-default index 0570e5148..2879dfba3 100755 --- a/hosts/common/programs/blast-ugjka/blast-to-default +++ b/hosts/common/programs/blast-ugjka/blast-to-default @@ -2,10 +2,7 @@ #!nix-shell -i python3 -p "python3.withPackages (ps: [ ])" -p blast-ugjka # vim: set filetype=python : -import ctypes import logging -import os -import signal import socket import subprocess @@ -19,50 +16,6 @@ DEVICE_MAP = { "[LG] webOS TV OLED55C9PUA": [ "-usewav" ], } -def set_pdeathsig(sig=signal.SIGTERM): - """ - helper function to ensure once parent process exits, its children processes will automatically die. - see: - see: - """ - libc = ctypes.CDLL("libc.so.6") - return libc.prctl(1, sig) - -MY_PID = None - -def reap_children(sig=None, frame=None): - global MY_PID - # reset SIGTERM handler to avoid recursing - signal.signal(signal.SIGTERM, signal.Handlers.SIG_DFL) - logger.info("killing all children (of pid %d)", MY_PID) - os.killpg(MY_PID, signal.SIGTERM) - -def reap_on_exit(): - """ - catch when the parent exits, and map that to SIGTERM for this process. - when this process receives SIGTERM, also terminate all descendent processes. - - this is done because: - 1. mpv invokes this, but (potentially) via the sandbox wrapper. - 2. when mpv exits, it `SIGKILL`s that sandbox wrapper. - 3. bwrap does not pass SIGKILL or SIGTERM to its child. - 4. hence, we neither receive that signal NOR can we pass it on simply by killing our immediate children - (since any bwrap'd children wouldn't pass that signal on...) - really, the proper fix would be on mpv's side: - - mpv should create a new process group when it launches a command, and kill that process group on exit. - or fix this in the sandbox wrapper: - - why *doesn't* bwrap forward the signals? - - there's --die-with-parent, but i can't apply that *system wide* and expect reasonably behavior - - """ - global MY_PID - MY_PID = os.getpid() - # create a new process group, pgid = gid - os.setpgid(MY_PID, MY_PID) - - set_pdeathsig(signal.SIGTERM) - signal.signal(signal.SIGTERM, reap_children) - def get_ranked_ip_addrs(): """ return the IP addresses most likely to be LAN addresses @@ -96,8 +49,6 @@ class BlastDriver: stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, - # this pdeathsig isn't necessary; seems it might result in leaked pulse outputs - # preexec_fn=set_pdeathsig ) self.blast_flags = list(blast_flags) self.receiver_names = [] @@ -202,15 +153,11 @@ def main(): logging.basicConfig() logging.getLogger().setLevel(logging.DEBUG) - reap_on_exit() - blast = try_blast() if blast is not None: logger.info("waiting until blast exits") blast.blast.wait() - reap_children() - if __name__ == "__main__": main() diff --git a/hosts/common/programs/blast-ugjka/default.nix b/hosts/common/programs/blast-ugjka/default.nix index e5e1e0ae3..6af7da7b8 100644 --- a/hosts/common/programs/blast-ugjka/default.nix +++ b/hosts/common/programs/blast-ugjka/default.nix @@ -44,7 +44,7 @@ in # might be possible to remove this, but kinda hard to see a clean way. "--sane-sandbox-keep-namespace" "pid" ]; - suggestedPrograms = [ "blast-ugjka" ]; + suggestedPrograms = [ "blast-ugjka" "sane-die-with-parent" ]; }; networking.firewall.allowedTCPPorts = lib.mkIf cfg.enabled [ 9000 ]; diff --git a/hosts/common/programs/mpv/sane_cast/main.lua b/hosts/common/programs/mpv/sane_cast/main.lua index 0bdf672bb..3ac1670e8 100644 --- a/hosts/common/programs/mpv/sane_cast/main.lua +++ b/hosts/common/programs/mpv/sane_cast/main.lua @@ -30,7 +30,12 @@ function invoke_go2tv_on_open_file(mode) invoke_go2tv(true, { mode, path }) end -mp.add_key_binding(nil, "blast", function() subprocess(false, { "blast-to-default" }) end) +-- invoke blast in a way where it dies when we die, because: +-- 1. when mpv exits, it `SIGKILL`s this toplevel subprocess. +-- 2. `blast-to-default` could be a sandbox wrapper. +-- 3. bwrap does not pass SIGKILL or SIGTERM to its child. +-- 4. hence, to properly kill blast, we have to kill all the descendants. +mp.add_key_binding(nil, "blast", function() subprocess(false, { "sane-die-with-parent", "--descendants", "--use-pgroup", "--catch-sigkill", "blast-to-default" }) end) mp.add_key_binding(nil, "go2tv-gui", function() invoke_go2tv(false, {}) end) mp.add_key_binding(nil, "go2tv-video", function() invoke_go2tv_on_open_file("-v") end) mp.add_key_binding(nil, "go2tv-stream", function() invoke_go2tv_on_open_file("-s") end) diff --git a/pkgs/additional/sane-die-with-parent/sane-die-with-parent b/pkgs/additional/sane-die-with-parent/sane-die-with-parent index a0d1d5827..e509162dd 100755 --- a/pkgs/additional/sane-die-with-parent/sane-die-with-parent +++ b/pkgs/additional/sane-die-with-parent/sane-die-with-parent @@ -7,13 +7,14 @@ USAGE: sane-die-with-parent [options...] [args ...] run `cmd` such that when the caller of sane-die-with-parent exits, `cmd` exits as well. OPTIONS: ---signal SIGKILL|SIGTERM: control the signal which is sent to child processes +--catch-sigkill: if this process is SIGKILL'd, also forward that to descendants/pgroup. +--descendents: run as a supervisor, and kill every process spawned below this one when the parent dies. + this is useful for running programs which also don't propagate the death signal, + such as `bubblewrap`. + without this, the default is to `exec` . +--signal SIGKILL|SIGTERM: control the signal which is sent to child processes. +--use-pgroup: kill children by killing the entire process group instead of walking down the hierarchy. --verbose ---descendents: run as a supervisor, and kill every process spawned below this one when the parent dies. - this is useful for running programs which also don't propagate the death signal, - such as `bubblewrap`. - without this, the default is to `exec` . ---use-pgroup: kill children by killing the entire process group instead of walking down the hierarchy """ import ctypes @@ -152,28 +153,46 @@ def main(): logging.basicConfig() args = sys.argv[1:] + catch_sigkill = False descendants = False - killsig = "sigterm" + killsig_, killsig = None, signal.SIGTERM use_pgroup = False verbose = False while args and args[0].startswith("--"): flag, args = args[0], args[1:] - if flag == "--descendants": + if flag == "--catch-sigkill": + catch_sigkill = True + elif flag == "--descendants": descendants = True elif flag == "--use-pgroup": use_pgroup = True elif flag == "--verbose": verbose = True elif flag == "--signal": - killsig, args = args[0], args[1:] + killsig_, args = args[0], args[1:] + killsig = getattr(signal, killsig_.upper()) else: assert False, f"unrecognized argument {flag!r}" cli = args - killsig = getattr(signal, killsig.upper()) if verbose: logging.getLogger().setLevel(logging.DEBUG) + if catch_sigkill: + nested_args = [ sys.argv[0] ] + if descendants: + nested_args += [ "--descendants" ] + descendants = True # it's less that we need the outer process to kill its descendants, so much as that it must *exist* + if killsig_: + nested_args += [ "--signal", killsig_ ] + if use_pgroup: + nested_args += [ "--use-pgroup" ] + use_pgroup = False # doesn't make sense for parent to use pgroups + if verbose: + nested_args += [ "--verbose" ] + + cli = nested_args + cli + if use_pgroup: PGID = os.getpid() # create a new process group, pgid = gid