fix(console): socketiod recovers when the console process ends

When the serial console process exited on its own (a bad token, an
unreachable host, a killed consolecallback), the read task died on EIO
while fd and child_pid stayed set: the child was a zombie and every later
connection wrote to the dead pty until socketiod was restarted. A child
that failed before exec also returned into the server code and sent its
traceback to the client.

Now the read loop treats EOF/EIO as the end: it reaps the child, closes
the pty, tells the clients "[console closed]" and disconnects them, so
the next connection starts a new console. The child always leaves with
os._exit, also without a token; output is decoded incrementally, and the
unused imports are gone.

console/socketiod

@@ -27,27 +27,19 @@ if not getattr(django_settings, "SERIAL_CONSOLE_ENABLED", False):
     )
     sys.exit(1)
 
-import re
-import tempfile
-import io
-import socket
-import socketio
+import codecs
 import pty
 import select
 import struct
 import fcntl
 import termios
 import signal
+import socketio
 import eventlet
-import atexit
-import tty
-import termios
-import libvirt
 
 # from six.moves import http_cookies as Cookie
 from http import cookies as Cookie
 from webvirtcloud.settings import SOCKETIO_PORT, SOCKETIO_HOST
-from vrtManager.connection import CONN_SSH, CONN_SOCKET
 import argparse
 
 
@@ -131,19 +123,51 @@ def set_winsize(fd, row, col, xpix=0, ypix=0):
     fcntl.ioctl(fd, termios.TIOCSWINSZ, winsize)
 
 
-def read_and_forward_pty_output():
+def stop_console():
+    """Stop the console process (if still running), reap it and close its pty."""
     global fd
+    global child_pid
+
+    if child_pid:
+        try:
+            os.kill(child_pid, signal.SIGKILL)
+        except ProcessLookupError:
+            pass
+        try:
+            os.waitpid(child_pid, 0)  # only the child this daemon started
+        except ChildProcessError:
+            pass
+    if fd is not None:
+        os.close(fd)
+    fd = None
+    child_pid = None
+
+
+def read_and_forward_pty_output():
     max_read_bytes = 1024 * 20
+    # a character may be split between two reads
+    decoder = codecs.getincrementaldecoder("utf-8")(errors="replace")
+    # The pty number may be reused by the next console; the pid identifies
+    # this one. Every yield (sleep, emit) may end it.
+    console_pid, console_fd = child_pid, fd
     while True:
         sio.sleep(0.01)
-        if fd:
-            timeout_sec = 0
-            (data_ready, _, _) = select.select([fd], [], [], timeout_sec)
-            if data_ready:
-                output = os.read(fd, max_read_bytes).decode()
-                sio.emit("pty_output", {"output": output})
-        else:
+        if child_pid != console_pid:
             return
+        (data_ready, _, _) = select.select([console_fd], [], [], 0)
+        if not data_ready:
+            continue
+        try:
+            output = os.read(console_fd, max_read_bytes)
+        except OSError:  # EIO: the console process ended
+            output = b""
+        if not output:
+            sio.emit("pty_output", {"output": decoder.decode(b"", final=True) + "\r\n[console closed]\r\n"})
+            stop_console()
+            for sid, _ in list(sio.manager.get_participants("/", None)):
+                sio.disconnect(sid)
+            return
+        sio.emit("pty_output", {"output": decoder.decode(output)})
 
 
 @sio.event
@@ -170,6 +194,7 @@ def connect(sid, environ):
     global fd
     global child_pid
 
+    token = None
     hcookie = environ.get("HTTP_COOKIE")
     if hcookie:
         cookie = Cookie.SimpleCookie()
@@ -178,7 +203,7 @@ def connect(sid, environ):
             try:
                 cookie.load(hcookie_part)
             except Cookie.CookieError:
-                logging.warn("Found malformed cookie")
+                logging.warning("Found malformed cookie")
             else:
                 if "token" in cookie:
                     token = cookie["token"].value
@@ -193,13 +218,18 @@ def connect(sid, environ):
     (child_pid, fd) = pty.fork()
 
     if child_pid == 0:
-        (instance, conn) = get_connection_infos(token)
-        uuid = conn.get_uuid()
-        uri = conn.wvm.getURI()
-
-        # Become the console process; returning would run this server's code
-        # in the child as well.
-        os.execv("conf/daemon/consolecallback", ["consolecallback", uri, uuid])
+        # Become the console process; never return, which would run this
+        # server's code in the child as well. Its output goes to the client.
+        try:
+            if token is None:
+                sys.stderr.write("No console token\n")
+            else:
+                (instance, conn) = get_connection_infos(token)
+                os.execv("conf/daemon/consolecallback", ["consolecallback", conn.wvm.getURI(), conn.get_uuid()])
+        except BaseException:
+            pass  # get_connection_infos logged it
+        finally:
+            os._exit(1)
     else:
         # this is the parent process fork.
         sio.start_background_task(target=read_and_forward_pty_output)
@@ -207,25 +237,9 @@ def connect(sid, environ):
 
 @sio.event
 def disconnect(sid):
-
-    global fd
-    global child_pid
-
-    # kill pty process
-    try:
-        os.kill(child_pid, signal.SIGKILL)
-        os.waitpid(child_pid, 0)  # only the child this daemon started
-    except ProcessLookupError:
-        pass
-    except ChildProcessError:
-        pass
-
-    # reset the variables
-    fd = None
-    child_pid = None
+    stop_console()
 
 
 app = socketio.WSGIApp(sio)
-import eventlet
 
 eventlet.wsgi.server(eventlet.listen((options.host, int(options.port))), app)

console/test_socketiod_child.py

@@ -0,0 +1,118 @@
+"""socketiod's console process ending on its own: the clients are told and
+disconnected, the process is reaped and the next connection starts a new one."""
+
+import os
+import socket
+import subprocess
+import sys
+import tempfile
+import threading
+import time
+import unittest
+from pathlib import Path
+
+try:
+    import socketio
+except ImportError:  # pragma: no cover
+    socketio = None
+
+ROOT = Path(__file__).resolve().parent.parent
+SOCKETIOD = ROOT / "console" / "socketiod"
+
+
+def free_port():
+    with socket.socket() as s:
+        s.bind(("127.0.0.1", 0))
+        return s.getsockname()[1]
+
+
+def children(pid):
+    """(pid, state) of the processes whose parent is pid."""
+    found = []
+    for entry in os.listdir("/proc"):
+        try:
+            with open("/proc/%s/stat" % entry) as f:
+                fields = f.read().rsplit(")", 1)[1].split()
+        except (OSError, IndexError):
+            continue
+        if int(fields[1]) == pid:
+            found.append((int(entry), fields[0]))
+    return found
+
+
+@unittest.skipIf(socketio is None, "python-socketio is not installed")
+@unittest.skipUnless(sys.platform.startswith("linux"), "reads /proc")
+class SocketiodChildExitTestCase(unittest.TestCase):
+    def setUp(self):
+        tmp = tempfile.TemporaryDirectory()
+        self.addCleanup(tmp.cleanup)
+        (Path(tmp.name) / "socketiod_child_settings.py").write_text(
+            "from webvirtcloud.settings import *  # noqa\n"
+            "SERIAL_CONSOLE_ENABLED = True\n"
+            "DATABASES = {'default': {'ENGINE': 'django.db.backends.sqlite3', 'NAME': %r}}\n"
+            % str(Path(tmp.name) / "db.sqlite3")
+        )
+        self.port = free_port()
+        env = dict(
+            os.environ,
+            DJANGO_SETTINGS_MODULE="socketiod_child_settings",
+            PYTHONPATH=os.pathsep.join([tmp.name, str(ROOT)]),
+        )
+        self.server = subprocess.Popen(
+            [sys.executable, str(SOCKETIOD), "-H", "127.0.0.1", "-p", str(self.port)],
+            cwd=ROOT,
+            env=env,
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+        )
+        self.addCleanup(self.stop_server)
+        end = time.monotonic() + 20
+        while time.monotonic() < end:
+            with socket.socket() as s:
+                if s.connect_ex(("127.0.0.1", self.port)) == 0:
+                    return
+            self.assertIsNone(self.server.poll(), "socketiod exited at start")
+            time.sleep(0.1)
+        self.fail("socketiod did not listen within 20 s")
+
+    def stop_server(self):
+        self.server.kill()
+        self.server.wait()
+
+    def open_console(self):
+        """Connect with a malformed token, so the console process fails at once;
+        returns the output and whether the server disconnected the client."""
+        client = socketio.Client(reconnection=False)
+        output = []
+        gone = threading.Event()
+        client.on("pty_output", lambda data: output.append(data["output"]))
+        client.on("disconnect", lambda *args: gone.set())
+        client.connect(
+            "http://127.0.0.1:%d" % self.port,
+            headers={"Cookie": "token=no-such-vm"},
+            transports=["polling"],
+        )
+        try:
+            gone.wait(15)
+        finally:
+            if not gone.is_set():
+                client.disconnect()
+        return "".join(output), gone.is_set()
+
+    def assert_no_child_left(self):
+        end = time.monotonic() + 5
+        while children(self.server.pid) and time.monotonic() < end:
+            time.sleep(0.05)
+        self.assertEqual(children(self.server.pid), [])
+
+    def test_the_client_is_told_and_the_next_connection_starts_again(self):
+        for attempt in (1, 2):
+            with self.subTest(attempt=attempt):
+                output, disconnected = self.open_console()
+                self.assertIn("console closed", output)
+                # no traceback of socketiod's own code (on Python 3.13 eventlet
+                # prints an ignored _after_fork error in every forked child)
+                self.assertNotIn('socketiod", line', output)
+                self.assertTrue(disconnected)
+                self.assert_no_child_left()
+                self.assertIsNone(self.server.poll())