[PATCH v2 2/3] prserv: make --start fail when the server does not come up

[Babanpreet Singh <bbnpreetsingh@gmail.com>]

Make `prserv --start` fail when the server can’t start, and clean up stale pidfiles on failure.

AI-Generated: Uses Claude (claude-fable-5-1)
Signed-off-by: Babanpreet Singh <bbnpreetsingh@gmail.com>
---
 lib/prserv/serv.py  | 53 +++++++++++++++++++++++++++++++++++++++------
 lib/prserv/tests.py | 14 ++++++++++++
 2 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index e17588630..6aba1c3e0 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -205,12 +205,13 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
         return {"readonly": self.server.read_only}
 
 class PRServer(bb.asyncrpc.AsyncServer):
-    def __init__(self, dbfile, read_only=False, upstream=None):
+    def __init__(self, dbfile, read_only=False, upstream=None, on_ready=None):
         super().__init__(logger)
         self.dbfile = dbfile
         self.table = None
         self.read_only = read_only
         self.upstream = upstream
+        self.on_ready = on_ready
 
     def accept_client(self, socket):
         return PRServerClient(socket, self)
@@ -226,6 +227,9 @@ class PRServer(bb.asyncrpc.AsyncServer):
         if self.upstream is not None:
             self.logger.info("And upstream PRServer: %s " % (self.upstream))
 
+        if self.on_ready is not None:
+            self.on_ready()
+
         return tasks
 
     async def stop(self):
@@ -253,16 +257,27 @@ class PRServSingleton(object):
 def run_as_daemon(func, pidfile, logfile):
     """
     See Advanced Programming in the UNIX, Sec 13.3
+
+    func runs in the daemon and is passed a callable to invoke once it is
+    ready to serve. Return the daemon's pid when that happens; raise
+    RuntimeError with the daemon's error if it fails first.
     """
+    readfd, writefd = os.pipe()
     try:
         pid = os.fork()
         if pid > 0:
+            os.close(writefd)
             os.waitpid(pid, 0)
+            with os.fdopen(readfd) as f:
+                report = f.read()
+            if report != "ready":
+                raise RuntimeError(report or "daemon exited before it was ready")
             #parent return instead of exit to give control
             return pid
     except OSError as e:
         raise Exception("%s [%d]" % (e.strerror, e.errno))
 
+    os.close(readfd)
     os.setsid()
     """
     fork again to make sure the daemon is not session leader,
@@ -316,9 +331,27 @@ def run_as_daemon(func, pidfile, logfile):
     with open(pidfile, "w") as pf:
         pf.write("%s\n" % pid)
 
-    func()
-    os.remove(pidfile)
-    os._exit(0)
+    def ready():
+        nonlocal writefd
+        os.write(writefd, b"ready")
+        os.close(writefd)
+        writefd = None
+
+    ret = 0
+    try:
+        func(ready)
+    except Exception as e:
+        import traceback
+        traceback.print_exc()
+        if writefd is not None:
+            os.write(writefd, str(e).encode())
+        ret = 1
+    try:
+        os.remove(pidfile)
+    except FileNotFoundError:
+        # --stop may have removed it already
+        pass
+    os._exit(ret)
 
 def start_daemon(dbfile, host, port, logfile, read_only=False, upstream=None):
     ip = socket.gethostbyname(host)
@@ -335,12 +368,18 @@ def start_daemon(dbfile, host, port, logfile, read_only=False, upstream=None):
         return 1
 
     dbfile = os.path.abspath(dbfile)
-    def daemon_main():
-        server = PRServer(dbfile, read_only=read_only, upstream=upstream)
+    logfile = os.path.abspath(logfile)
+    def daemon_main(ready):
+        server = PRServer(dbfile, read_only=read_only, upstream=upstream, on_ready=ready)
         server.start_tcp_server(ip, port)
         server.serve_forever()
 
-    run_as_daemon(daemon_main, pidfile, os.path.abspath(logfile))
+    try:
+        run_as_daemon(daemon_main, pidfile, logfile)
+    except RuntimeError as e:
+        sys.stderr.write("Failed to start PRServer on %s:%s: %s\nSee %s for details.\n"
+                         % (ip, port, e, logfile))
+        return 1
     return 0
 
 def stop_daemon(host, port):
diff --git a/lib/prserv/tests.py b/lib/prserv/tests.py
index f3b2aae45..f1e4f12c8 100644
--- a/lib/prserv/tests.py
+++ b/lib/prserv/tests.py
@@ -7,6 +7,7 @@
 
 from . import create_server, create_client, increase_revision, revision_greater, revision_smaller, _revision_greater_or_equal
 import prserv.db as db
+from prserv.serv import PIDPREFIX
 from bb.asyncrpc import InvokeError
 import logging
 import os
@@ -383,6 +384,19 @@ class ScriptTests(unittest.TestCase):
     def tearDownClass(cls):
         cls.temp_dir.cleanup()
 
+    def test_0_start_bitbake_prserv_port_in_use(self):
+        with socket.socket() as s:
+            s.bind(("0.0.0.0", 0))
+            s.listen()
+            port = str(s.getsockname()[1])
+            result = subprocess.run([BIN_DIR / "bitbake-prserv", "--start", "-f", self.dbfile, "--port", port],
+                                    capture_output=True, text=True)
+        self.assertNotEqual(result.returncode, 0,
+                            "--start reported success with the port in use")
+        self.assertIn("Failed to start PRServer", result.stderr)
+        self.assertFalse(os.path.exists(PIDPREFIX % ("0.0.0.0", port)),
+                         "pidfile left behind by a server that never came up")
+
     def test_1_start_bitbake_prserv(self):
         try:
             subprocess.check_call([BIN_DIR / "bitbake-prserv", "--start", "-f", self.dbfile, "--port", self.port])
-- 
2.43.0