blast-to-default: leverage sane-die-with-parent

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: <https://stackoverflow.com/a/43152455>
-    see: <https://www.man7.org/linux/man-pages/man2/prctl.2.html>
-    """
-    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
-      <https://github.com/containers/bubblewrap/issues/529>
-    """
-    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()

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 ];

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)

pkgs/additional/sane-die-with-parent/sane-die-with-parent

@@ -7,13 +7,14 @@ USAGE: sane-die-with-parent [options...] <cmd> [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` <cmd>.
+--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` <cmd>.
---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