From a570635d239f46c5a9758a8a50f913f376943fbf Mon Sep 17 00:00:00 2001
From: Marek Miklewicz <marek@miklewicz.pl>
Date: Sat, 4 Jul 2026 21:59:13 +0200
Subject: [PATCH] Fix busy-loop in withQueueLock() on non-EEXIST mkdir failure

If mkdir(queue.lock) failed for a reason other than "already exists"
(e.g. a permissions or disk problem on the data directory), is_dir()
was false, so the loop `continue`d before ever reaching the tries>=40
retry-limit check or the usleep(), spinning at full CPU forever
instead of failing with a clear error.

Move the staleness check inside an is_dir() guard instead of using
continue to skip past the retry-limit logic, so the bounded-retry
behavior applies regardless of why mkdir failed.
---
 exec/lib/BackupQueueService.php  |  11 ++-
 tests/backup_queue_lock_test.php | 114 +++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+), 6 deletions(-)
 create mode 100644 tests/backup_queue_lock_test.php

diff --git a/exec/lib/BackupQueueService.php b/exec/lib/BackupQueueService.php
index 405746e..35500c5 100644
--- a/exec/lib/BackupQueueService.php
+++ b/exec/lib/BackupQueueService.php
@@ -1295,13 +1295,12 @@ final class BackupQueueService
         $tries = 0;
         while (!@mkdir($lockDir, 0700)) {
             $tries++;
-            if (!is_dir($lockDir)) {
-                continue;
-            }
 
-            $mtime = (int)@filemtime($lockDir);
-            if ($mtime > 0 && (time() - $mtime) > 120) {
-                @rmdir($lockDir);
+            if (is_dir($lockDir)) {
+                $mtime = (int)@filemtime($lockDir);
+                if ($mtime > 0 && (time() - $mtime) > 120) {
+                    @rmdir($lockDir);
+                }
             }
 
             if ($tries >= 40) {
diff --git a/tests/backup_queue_lock_test.php b/tests/backup_queue_lock_test.php
new file mode 100644
index 0000000..edabac6
--- /dev/null
+++ b/tests/backup_queue_lock_test.php
@@ -0,0 +1,114 @@
+<?php
+declare(strict_types=1);
+
+$pluginTempRoot = sys_get_temp_dir() . '/altmysql-plugin-root-' . bin2hex(random_bytes(4));
+mkdir($pluginTempRoot . '/data/jobs', 0700, true);
+define('PLUGIN_ROOT', $pluginTempRoot);
+
+$pluginDir = dirname(__DIR__);
+require_once $pluginDir . '/exec/lib/Settings.php';
+require_once $pluginDir . '/exec/lib/DirectAdminUser.php';
+require_once $pluginDir . '/exec/lib/BackupQueueService.php';
+
+function fail(string $message): void
+{
+    fwrite(STDERR, "FAIL: {$message}\n");
+    exit(1);
+}
+
+function load_settings(string $contents): Settings
+{
+    $path = tempnam(sys_get_temp_dir(), 'altmysql-settings-');
+    if ($path === false) {
+        fail('cannot create temp settings file');
+    }
+    file_put_contents($path, $contents);
+    $settings = Settings::load($path);
+    unlink($path);
+    return $settings;
+}
+
+function make_da_user(Settings $settings, string $username): DirectAdminUser
+{
+    $class = new ReflectionClass(DirectAdminUser::class);
+    /** @var DirectAdminUser $user */
+    $user = $class->newInstanceWithoutConstructor();
+    foreach ([
+        'username' => $username,
+        'prefix' => $username . '_',
+        'userConf' => [],
+        'packageConf' => [],
+        'settings' => $settings,
+    ] as $property => $value) {
+        $prop = $class->getProperty($property);
+        $prop->setAccessible(true);
+        $prop->setValue($user, $value);
+    }
+    return $user;
+}
+
+if (!function_exists('pcntl_fork')) {
+    fail('pcntl extension is required to safely test withQueueLock() without risking a hang');
+}
+
+// Make the jobs/ directory read-only so mkdir(queue.lock) fails for a reason
+// OTHER than "already exists" (EACCES, not EEXIST) - this is the branch that
+// must never busy-loop forever.
+chmod($pluginTempRoot . '/data/jobs', 0500);
+
+$pid = pcntl_fork();
+if ($pid === -1) {
+    fail('could not fork to safely bound the withQueueLock() call');
+}
+
+if ($pid === 0) {
+    // Child: attempt the call. If it hangs, the parent will kill us after
+    // the deadline below and the test fails. If it throws (correct fixed
+    // behavior), exit 42. Any other outcome exits 1.
+    $settings = load_settings('');
+    $daUser = make_da_user($settings, 'demo');
+    $queue = new BackupQueueService($settings, $daUser);
+
+    $class = new ReflectionClass(BackupQueueService::class);
+    $method = $class->getMethod('withQueueLock');
+    $method->setAccessible(true);
+
+    try {
+        $method->invoke($queue, static fn () => 'unreachable');
+        exit(1);
+    } catch (RuntimeException $e) {
+        exit(42);
+    } catch (Throwable $e) {
+        exit(1);
+    }
+}
+
+$deadline = microtime(true) + 5.0;
+$status = 0;
+$exited = false;
+while (microtime(true) < $deadline) {
+    $result = pcntl_waitpid($pid, $status, WNOHANG);
+    if ($result === $pid) {
+        $exited = true;
+        break;
+    }
+    usleep(50000);
+}
+
+if (!$exited) {
+    posix_kill($pid, SIGKILL);
+    pcntl_waitpid($pid, $status);
+    @chmod($pluginTempRoot . '/data/jobs', 0700);
+    exec('rm -rf ' . escapeshellarg($pluginTempRoot));
+    fail('withQueueLock() busy-looped instead of giving up after a bounded number of retries (killed after 5s)');
+}
+
+@chmod($pluginTempRoot . '/data/jobs', 0700);
+exec('rm -rf ' . escapeshellarg($pluginTempRoot));
+
+$exitCode = pcntl_wexitstatus($status);
+if ($exitCode !== 42) {
+    fail("withQueueLock() did not fail with the expected RuntimeException (child exit code: $exitCode)");
+}
+
+echo "backup_queue_lock_test: OK\n";