From 27fa6af30d21799f4e3ad43a9c7bdd3cbc72cbe2 Mon Sep 17 00:00:00 2001
From: Bogdan <matolq@gmail.com>
Date: Tue, 10 Nov 2026 21:35:02 +0100
Subject: [PATCH 1/2] fix: make database job reservation atomic and always
 close transaction

---
 src/Models/QueueJobModel.php | 41 ++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/src/Models/QueueJobModel.php b/src/Models/QueueJobModel.php
index 2e13c8b..9fb11e0 100644
--- a/src/Models/QueueJobModel.php
+++ b/src/Models/QueueJobModel.php
@@ -160,23 +160,36 @@ private function popWithLock(string $name, array $priority): ?QueueJob
         $builder = $this->setPriority($builder, $priority);
         $sql     = $builder->getCompiledSelect();
 
-        $query = $this->db->query($this->skipLocked($sql));
-        if ($query === false) {
-            $this->db->transComplete();
+        try {
+            $query = $this->db->query($this->skipLocked($sql));
 
-            return null;
-        }
-        /** @var QueueJob|null $row */
-        $row = $query->getCustomRowObject(0, QueueJob::class);
+            if ($query === false) {
+                return null;
+            }
 
-        if ($row !== null) {
-            // Change status
-            $this->update($row->id, ['status' => Status::RESERVED->value]);
-        }
-        // Complete transaction
-        $this->db->transComplete();
+            /** @var QueueJob|null $row */
+            $row = $query->getCustomRowObject(0, QueueJob::class);
 
-        return $row;
+            if ($row === null) {
+                return null;
+            }
+
+            // Change status. The job must not be processed unless the
+            // reservation is confirmed by exactly one affected row.
+            $updated = $this->update($row->id, ['status' => Status::RESERVED->value]);
+
+            if ($updated === false || $this->db->affectedRows() !== 1) {
+                $this->db->transRollback();
+
+                return null;
+            }
+
+            return $row;
+        } finally {
+            // Always close the transaction, even when a query fails or the
+            // reservation cannot be confirmed.
+            $this->db->transComplete();
+        }
     }
 
     /**

From 1eb0b52cf31124781fb598fb67ba0f8cd7c23263 Mon Sep 17 00:00:00 2001
From: Bogdan <matolq@gmail.com>
Date: Tue, 10 Nov 2026 21:53:12 +0100
Subject: [PATCH 2/2] test: verify transaction rollback and closure in
 popWithLock on failure

---
 tests/Models/QueueJobModelTest.php | 66 ++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/tests/Models/QueueJobModelTest.php b/tests/Models/QueueJobModelTest.php
index 00ef79f..dec786d 100644
--- a/tests/Models/QueueJobModelTest.php
+++ b/tests/Models/QueueJobModelTest.php
@@ -132,4 +132,70 @@ public function testSetPriorityEscapesPriorityValues(): void
         $this->assertStringContainsString($model->db->escape($priority['priority_key']), $sql);
         $this->assertStringNotContainsString('priority_key', $sql);
     }
+
+    public function testPopWithLockReturnsNullAndClosesTransactionWhenQueryFails(): void
+    {
+        $model      = model(QueueJobModel::class);
+        $oldDBDebug = $this->getPrivateProperty($model->db, 'DBDebug');
+        $this->setPrivateProperty($model->db, 'DBDebug', false);
+
+        try {
+            $testModel = new class ($model->db) extends QueueJobModel {
+                protected $table = 'non_existent_table_xyz';
+            };
+            $method = $this->getPrivateMethodInvoker($testModel, 'popWithLock');
+
+            $result = $method('queue1', ['default']);
+
+            $this->assertNull($result);
+            $this->assertSame(0, $this->getPrivateProperty($testModel->db, 'transDepth'));
+        } finally {
+            $this->setPrivateProperty($model->db, 'DBDebug', $oldDBDebug);
+        }
+    }
+
+    public function testPopWithLockRollsBackAndReturnsNullWhenUpdateFails(): void
+    {
+        $existing = model(QueueJobModel::class);
+        $model    = $this->getMockBuilder(QueueJobModel::class)
+            ->setConstructorArgs([$existing->db])
+            ->onlyMethods(['update'])
+            ->getMock();
+        $model->method('update')->willReturn(false);
+
+        $method = $this->getPrivateMethodInvoker($model, 'popWithLock');
+        $result = $method('queue1', ['default']);
+
+        $this->assertNull($result);
+        $this->assertSame(0, $this->getPrivateProperty($model->db, 'transDepth'));
+        $this->seeInDatabase('queue_jobs', [
+            'id'     => 2,
+            'status' => Status::PENDING->value,
+        ]);
+    }
+
+    public function testPopWithLockRollsBackAndReturnsNullWhenAffectedRowsIsNotOne(): void
+    {
+        $existing = model(QueueJobModel::class);
+        $model    = $this->getMockBuilder(QueueJobModel::class)
+            ->setConstructorArgs([$existing->db])
+            ->onlyMethods(['update'])
+            ->getMock();
+        $model->method('update')->willReturnCallback(static function () use ($model): bool {
+            // Update 0 rows so affectedRows() is 0 instead of 1
+            $model->db->table('queue_jobs')->where('id', 999999)->update(['status' => 99]);
+
+            return true;
+        });
+
+        $method = $this->getPrivateMethodInvoker($model, 'popWithLock');
+        $result = $method('queue1', ['default']);
+
+        $this->assertNull($result);
+        $this->assertSame(0, $this->getPrivateProperty($model->db, 'transDepth'));
+        $this->seeInDatabase('queue_jobs', [
+            'id'     => 2,
+            'status' => Status::PENDING->value,
+        ]);
+    }
 }