diff mbox series

server/process: Fix lock owner prefix check during shutdown

Message ID 20260930194306.8490-1-eng.redamaher@gmail.com
State New
Headers show
Series server/process: Fix lock owner prefix check during shutdown | expand

Commit Message

Reda Maher Sept. 30, 2026, 7:43 p.m. UTC 
Pass a tuple to str.startswith() when checking a contended lock during shutdown. The list raises TypeError before the server can retry its own lock or recognize a replacement server.

Add a deterministic shutdown regression test covering PID-only and XML-RPC lock records, including a replacement PID sharing the old PID's prefix. Verified failure before the fix and success with bitbake-selftest bb.tests.server afterwards.

AI-Generated: Uses GitHub Copilot

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Reda Maher <eng.redamaher@gmail.com>
---
 bin/bitbake-selftest     |  1 +
 lib/bb/server/process.py |  2 +-
 lib/bb/tests/server.py   | 57 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 lib/bb/tests/server.py
diff mbox series

Patch

diff --git a/bin/bitbake-selftest b/bin/bitbake-selftest
index 855abb1f0..e5a68ba1d 100755
--- a/bin/bitbake-selftest
+++ b/bin/bitbake-selftest
@@ -35,6 +35,7 @@  tests = ["bb.tests.codeparser",
          "bb.tests.fetch_import",
          "bb.tests.parse",
          "bb.tests.runqueue",
+         "bb.tests.server",
          "bb.tests.setup",
          "bb.tests.siggen",
          "bb.tests.utils",
diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index d0f73590c..a103b0987 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -377,7 +377,7 @@  class ProcessServer():
                 lock = bb.utils.lockfile(lockfile, shared=False, retry=False, block=False)
                 if not lock:
                     newlockcontents = get_lock_contents(lockfile)
-                    if not newlockcontents[0].startswith([f"{os.getpid()}\n", f"{os.getpid()} "]):
+                    if not newlockcontents[0].startswith((f"{os.getpid()}\n", f"{os.getpid()} ")):
                         # A new server was started, the lockfile contents changed, we can exit
                         serverlog("Lockfile now contains different contents, exiting: " + str(newlockcontents))
                         return
diff --git a/lib/bb/tests/server.py b/lib/bb/tests/server.py
new file mode 100644
index 000000000..a1ec76e19
--- /dev/null
+++ b/lib/bb/tests/server.py
@@ -0,0 +1,57 @@ 
+#
+# BitBake Tests for server/process.py
+#
+# SPDX-License-Identifier: GPL-2.0-only
+#
+
+import os
+import tempfile
+import unittest
+from unittest.mock import Mock, patch
+
+from bb.server.process import ProcessServer
+
+
+class ProcessServerTests(unittest.TestCase):
+    def test_shutdown_contended_lock(self):
+        pid = os.getpid()
+        for owner in (str(pid), str(pid) + "0"):
+            for suffix in ("\n", " 127.0.0.1:12345\n"):
+                with self.subTest(owner=owner, suffix=suffix):
+                    with tempfile.TemporaryDirectory() as tmpdir:
+                        lockname = os.path.join(tmpdir, "bitbake.lock")
+                        sockname = os.path.join(tmpdir, "bitbake.sock")
+                        with open(lockname, "w") as stream:
+                            stream.write(owner + suffix)
+                        with open(sockname, "w"):
+                            pass
+
+                        lock = Mock()
+                        sock = Mock()
+                        server = ProcessServer(lock, lockname, sock, sockname,
+                                               0, (None, None))
+                        server.cooker = Mock()
+                        server.quit = True
+                        acquired_lock = Mock()
+                        own_lock = owner == str(pid)
+                        attempts = [None, acquired_lock] if own_lock else [None]
+
+                        with patch("bb.server.process.bb.utils.set_process_name"), \
+                             patch("bb.server.process.os.path.exists", return_value=True), \
+                             patch("bb.server.process.bb.utils.lockfile", side_effect=attempts) as acquire, \
+                             patch("bb.server.process.bb.utils.unlockfile") as unlock, \
+                             patch("bb.server.process.time.sleep") as sleep, \
+                             patch("bb.server.process.serverlog"):
+                            server.main()
+
+                        lock.close.assert_called_once_with()
+                        sock.close.assert_called_once_with()
+                        server.cooker.shutdown.assert_called_once_with(True, idle=False)
+                        self.assertEqual(acquire.call_count, 2 if own_lock else 1)
+                        acquire.assert_called_with(lockname, shared=False, retry=False, block=False)
+                        if own_lock:
+                            sleep.assert_called_once_with(0.1)
+                            unlock.assert_called_once_with(acquired_lock)
+                        else:
+                            sleep.assert_not_called()
+                            unlock.assert_not_called()