fix: stop ignoring SIGCHLD process-wide

The web app (wsgi.py), novncd and socketiod set SIGCHLD to SIG_IGN at
import. The kernel then reaps every child at once, so subprocess loses
the exit status and a failing command reports 0, in the server and in
anything else run in that process: the test suite had to reset the
signal in two tests.

It was added in 2023 against one defunct bash per gunicorn worker and
per novncd. That bash came from the settings template of the time,
which ran .dec_ldap_pwd.sh with Popen and never waited for it; the
template stopped doing that in 561fedf. A settings.py still holding
that line leaves at most one zombie per process, not a growing number.

- wsgi.py, novncd, socketiod: the global SIG_IGN is removed. subprocess
  reaps its own children, websockify installs its own SIGCHLD reaper
  for novncd's connection processes, and novncd's SSH tunnels wait for
  their ssh process.
- socketiod: the pty child execs consolecallback instead of running it
  and then returning into the server's code; disconnect reaps that
  child (waitpid) instead of any child (wait).
- The settings template drops its unused subprocess import; the two
  test workarounds are removed.

New tests load the web app and novncd in a fresh interpreter and check
that SIGCHLD is untouched and a failing command still returns 1; both
fail on the previous code. A patched novncd in the test container
served five connections and left no child behind.

console/novncd

@@ -5,9 +5,6 @@ import sys
 import logging
 import django
 
-import signal
-signal.signal(signal.SIGCHLD, signal.SIG_IGN)
-
 DIR_PATH = os.path.dirname(os.path.abspath(__file__))
 ROOT_PATH = os.path.abspath(os.path.join(DIR_PATH, "..", ""))
 os.environ.setdefault("DJANGO_SETTINGS_MODULE", "webvirtcloud.settings")

console/socketiod

@@ -4,9 +4,6 @@ import sys
 import logging
 import django
 
-import signal
-signal.signal(signal.SIGCHLD, signal.SIG_IGN)
-
 DIR_PATH = os.path.dirname(os.path.abspath(__file__))
 ROOT_PATH = os.path.abspath(os.path.join(DIR_PATH, "..", ""))
 os.environ.setdefault("DJANGO_SETTINGS_MODULE", "webvirtcloud.settings")
@@ -37,7 +34,6 @@ import socket
 import socketio
 import pty
 import select
-import subprocess
 import struct
 import fcntl
 import termios
@@ -201,7 +197,9 @@ def connect(sid, environ):
         uuid = conn.get_uuid()
         uri = conn.wvm.getURI()
 
-        subprocess.run(["conf/daemon/consolecallback", uri, uuid])
+        # Become the console process; returning would run this server's code
+        # in the child as well.
+        os.execv("conf/daemon/consolecallback", ["consolecallback", uri, uuid])
     else:
         # this is the parent process fork.
         sio.start_background_task(target=read_and_forward_pty_output)
@@ -216,7 +214,7 @@ def disconnect(sid):
     # kill pty process
     try:
         os.kill(child_pid, signal.SIGKILL)
-        os.wait()
+        os.waitpid(child_pid, 0)  # only the child this daemon started
     except ProcessLookupError:
         pass
     except ChildProcessError:

console/test_socketiod_disabled.py

@@ -1,5 +1,4 @@
 import os
-import signal
 import subprocess
 import sys
 import tempfile
@@ -28,9 +27,6 @@ class SocketiodDisabledTestCase(unittest.TestCase):
                 DJANGO_SETTINGS_MODULE="socketiod_disabled_settings",
                 PYTHONPATH=os.pathsep.join([tmp, str(ROOT)]),
             )
-            # Other test modules import novncd, which sets SIGCHLD to SIG_IGN in
-            # this process; with that, the child's exit status is lost (always 0).
-            previous_sigchld = signal.signal(signal.SIGCHLD, signal.SIG_DFL)
             try:
                 result = subprocess.run(
                     [sys.executable, str(SOCKETIOD)],
@@ -42,7 +38,5 @@ class SocketiodDisabledTestCase(unittest.TestCase):
                 )
             except subprocess.TimeoutExpired:
                 self.fail("socketiod kept running although SERIAL_CONSOLE_ENABLED=False")
-            finally:
-                signal.signal(signal.SIGCHLD, previous_sigchld)
         self.assertNotEqual(result.returncode, 0, result.stdout + result.stderr)
         self.assertIn("SERIAL_CONSOLE_ENABLED", result.stderr)

instances/test_gstfsd.py

@@ -1,6 +1,5 @@
 import json
 import os
-import signal
 import socket
 import subprocess
 import sys
@@ -51,9 +50,6 @@ class GstfsdTestCase(unittest.TestCase):
     """gstfsd hardening."""
 
     def setUp(self):
-        # novncd (imported by other tests) sets SIGCHLD to SIG_IGN, which hides exit codes.
-        previous = signal.signal(signal.SIGCHLD, signal.SIG_DFL)
-        self.addCleanup(signal.signal, signal.SIGCHLD, previous)
         self.tmp = tempfile.TemporaryDirectory()
         self.addCleanup(self.tmp.cleanup)
         Path(self.tmp.name, "guestfs.py").write_text(FAKE_GUESTFS)

webvirtcloud/settings.py.template

@@ -4,7 +4,6 @@ Django settings for webvirtcloud project.
 """
 
 import os
-import subprocess
 try:
     import ldap
     from django_auth_ldap.config import LDAPSearch, NestedActiveDirectoryGroupType

webvirtcloud/test_child_processes.py

@@ -0,0 +1,39 @@
+"""Loading the web app or novncd must not change how child processes are reaped.
+
+With SIGCHLD ignored process-wide, the kernel reaps every child at once and
+subprocess loses its exit status: a failing command reports 0.
+"""
+
+import subprocess
+import sys
+import unittest
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parent.parent
+
+CHECK = """
+import signal, subprocess
+assert signal.getsignal(signal.SIGCHLD) == signal.SIG_DFL, signal.getsignal(signal.SIGCHLD)
+print(subprocess.run(["false"]).returncode)
+"""
+
+LOAD_NOVNCD = """
+import importlib.machinery, importlib.util
+loader = importlib.machinery.SourceFileLoader("novncd", "console/novncd")
+loader.exec_module(importlib.util.module_from_spec(importlib.util.spec_from_loader("novncd", loader)))
+"""
+
+
+class ChildProcessTestCase(unittest.TestCase):
+    def run_python(self, code):
+        result = subprocess.run(
+            [sys.executable, "-c", code], cwd=ROOT, capture_output=True, text=True, timeout=60
+        )
+        self.assertEqual(result.returncode, 0, result.stderr)
+        return result.stdout.strip().splitlines()[-1]
+
+    def test_failing_command_is_nonzero_after_loading_the_web_app(self):
+        self.assertEqual(self.run_python("import webvirtcloud.wsgi\n" + CHECK), "1")
+
+    def test_failing_command_is_nonzero_after_loading_novncd(self):
+        self.assertEqual(self.run_python(LOAD_NOVNCD + CHECK), "1")

webvirtcloud/wsgi.py

@@ -7,14 +7,6 @@ For more information on this file, see
 https://docs.djangoproject.com/en/4.2/howto/deployment/wsgi/
 """
 
-import signal
-import threading
-
-# Reap child processes to avoid zombies. Signal handlers can only be set from
-# the main thread; runserver imports this module from a worker thread.
-if threading.current_thread() is threading.main_thread():
-    signal.signal(signal.SIGCHLD, signal.SIG_IGN)
-
 import os
 
 from django.core.wsgi import get_wsgi_application