Improve withinSessionLevelLock: invoke callback only on successful lo…

adr/adr-003-within-session-lock-exception-safety.md

@@ -56,8 +56,17 @@ PHP sets `previous` only in the exception constructor — there is no `addPrevio
 
 The library has no logger dependency and adding one for a single edge case is not justified. Users who need visibility into release failures can wrap `withinSessionLevelLock` in their own try/catch or use `acquireSessionLevelLock` / `releaseSessionLevelLock` directly.
 
+## Known limitation: callback result is lost on release failure
+
+When the callback succeeds but `releaseSessionLevelLock()` throws, `LockReleaseException` is thrown and the callback's return value is discarded. The caller receives no indication of the callback result.
+
+This is a consequence of PHP's `finally` semantics — a `throw` in `finally` prevents the `try` block's `return` from completing. The `LockReleaseException` itself serves as a reliable signal that the callback completed successfully (it is only thrown when `$exception === null`), but the return value is not preserved.
+
+A future improvement could introduce a dedicated exception subclass (e.g. `WithinSessionLockReleaseException`) that carries the callback result. For now, users who need the callback result in this scenario should use `acquireSessionLevelLock()` / `releaseSessionLevelLock()` directly.
+
 ## Consequences
 
 - Original exceptions from user callbacks are never masked by release failures.
 - `PG_ADVISORY_UNLOCK` is not called when the lock was never acquired, preventing reentrant counter corruption.
 - Release failures are silently suppressed when a callback exception is already in flight — acceptable trade-off given no logger.
+- Callback return value is lost when release fails — acceptable trade-off; low-level API is available for callers who need it.

src/Postgres/LockHandle/WithinSessionLevelLockHandle.php

@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of PHP DB Locker.
+ *
+ * (c) Anton Komarev <anton@komarev.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Cog\DbLocker\Postgres\LockHandle;
+
+/**
+ * @template TReturn
+ */
+final class WithinSessionLevelLockHandle
+{
+    /**
+     * @param TReturn $result
+     */
+    public function __construct(
+        public readonly bool $wasAcquired,
+        public readonly mixed $result,
+    ) {}
+}

src/Postgres/PostgresAdvisoryLocker.php

@@ -19,6 +19,7 @@
 use Cog\DbLocker\Postgres\Enum\PostgresLockAccessModeEnum;
 use Cog\DbLocker\Postgres\Enum\PostgresLockLevelEnum;
 use Cog\DbLocker\Postgres\LockHandle\SessionLevelLockHandle;
+use Cog\DbLocker\Postgres\LockHandle\WithinSessionLevelLockHandle;
 use Cog\DbLocker\Postgres\LockHandle\TransactionLevelLockHandle;
 use Cog\DbLocker\TimeoutDuration;
 
@@ -53,24 +54,28 @@ public function acquireTransactionLevelLock(
     }
 
     /**
-     * Acquires a session-level advisory lock and ensures its release after executing the callback.
+     * Acquires a session-level advisory lock, executes the callback only if acquired, and ensures release.
+     *
+     * The callback is invoked only when the lock is successfully acquired. If the lock
+     * is not acquired (held by another process), the callback is not called and
+     * a handle with `wasAcquired = false` is returned.
      *
      * This method guarantees that the lock is released even if an exception is thrown during execution.
      * Useful for safely wrapping critical sections that require locking.
      *
-     * If the lock was not acquired (i.e., `wasAcquired` is `false`), it is up to the callback
-     * to decide how to handle the situation (e.g., retry, throw, log, or silently skip).
-     *
      * ⚠️ Transaction-level advisory locks are strongly preferred whenever possible,
      * as they are automatically released at the end of a transaction and are less error-prone.
      * Use session-level locks only when transactional context is not available.
      *
-     * @param callable(SessionLevelLockHandle): TReturn $callback A callback that receives the lock handle.
+     * @param callable(): TReturn $callback A callback to execute while the lock is held.
      * @param TimeoutDuration $timeoutDuration Maximum wait time. Use TimeoutDuration::zero() for an immediate (non-blocking) attempt.
-     * @return TReturn The return value of the callback.
+     * @return WithinSessionLevelLockHandle<TReturn|null> Handle with wasAcquired and result of the callback (null if not acquired).
      *
      * @throws LockAcquireException If a database error occurs during lock acquisition. NOT thrown for normal lock contention.
      * @throws LockReleaseException If a database error occurs during lock release (only thrown if no other exception occurred during callback execution).
+     *     ⚠️ A LockReleaseException guarantees that the callback has completed successfully,
+     *     but the callback's return value is lost. If you need access to the callback result
+     *     in this scenario, use acquireSessionLevelLock() / releaseSessionLevelLock() directly.
      *
      * @see acquireTransactionLevelLock() for preferred locking strategy.
      *
@@ -82,32 +87,42 @@ public function withinSessionLevelLock(
         callable $callback,
         TimeoutDuration $timeoutDuration,
         PostgresLockAccessModeEnum $accessMode = PostgresLockAccessModeEnum::Exclusive,
-    ): mixed {
+    ): WithinSessionLevelLockHandle {
         $lockHandle = $this->acquireSessionLevelLock(
             dbConnection: $dbConnection,
             key: $key,
             timeoutDuration: $timeoutDuration,
             accessMode: $accessMode,
         );
 
+        if (!$lockHandle->wasAcquired) {
+            return new WithinSessionLevelLockHandle(
+                wasAcquired: false,
+                result: null,
+            );
+        }
+
         $exception = null;
         try {
-            return $callback($lockHandle);
+            $result = $callback();
+
+            return new WithinSessionLevelLockHandle(
+                wasAcquired: true,
+                result: $result,
+            );
         } catch (\Throwable $e) {
             $exception = $e;
             throw $e;
         } finally {
-            if ($lockHandle->wasAcquired) {
-                try {
-                    $this->releaseSessionLevelLock(
-                        dbConnection: $dbConnection,
-                        key: $key,
-                        accessMode: $accessMode,
-                    );
-                } catch (\Throwable $releaseException) {
-                    if ($exception === null) {
-                        throw $releaseException;
-                    }
+            try {
+                $this->releaseSessionLevelLock(
+                    dbConnection: $dbConnection,
+                    key: $key,
+                    accessMode: $accessMode,
+                );
+            } catch (\Throwable $releaseException) {
+                if ($exception === null) {
+                    throw $releaseException;
                 }
             }
         }

test/Integration/Postgres/PostgresAdvisoryLockerTest.php

@@ -843,7 +843,7 @@ public function testItCanExecuteCodeWithinSessionLock(
         $y = 3;
 
         // WHEN: Executing code within a session-level lock using callback
-        $result = $locker->withinSessionLevelLock(
+        $handle = $locker->withinSessionLevelLock(
             new PdoConnectionAdapter($dbConnection),
             $lockKey,
             function () use ($dbConnection, $lockKey, $accessMode, $x, $y): int {
@@ -857,7 +857,8 @@ function () use ($dbConnection, $lockKey, $accessMode, $x, $y): int {
         );
 
         // THEN: Callback should execute successfully and lock should be auto-released
-        $this->assertSame(5, $result);
+        $this->assertTrue($handle->wasAcquired);
+        $this->assertSame(5, $handle->result);
         $this->assertPgAdvisoryLocksCount(0);
         $this->assertPgAdvisoryLockMissingInConnection($dbConnection, $lockKey);
     }
@@ -902,11 +903,13 @@ function () use ($dbConnection): never {
             // THEN: The original exception from callback should be preserved, not masked by release failure
             $this->assertSame('Original error from callback', $e->getMessage());
         } catch (\PDOException $e) {
-            $this->fail('PDOException from release should not mask the original RuntimeException: ' . $e->getMessage());
+            $this->fail(
+                'PDOException from release should not mask the original RuntimeException: ' . $e->getMessage(),
+            );
         }
     }
 
-    public function testWithinSessionLevelLockDoesNotReleaseWhenLockWasNotAcquired(): void
+    public function testWithinSessionLevelLockDoesNotCallCallbackWhenLockWasNotAcquired(): void
     {
         // GIVEN: A lock already held by another connection so the second cannot acquire it
         $locker = $this->initLocker();
@@ -918,22 +921,60 @@ public function testWithinSessionLevelLockDoesNotReleaseWhenLockWasNotAcquired()
             $lockKey,
             TimeoutDuration::zero(),
         );
+        $callbackExecuted = false;
 
         // WHEN: withinSessionLevelLock is called on second connection (lock not acquired)
-        $locker->withinSessionLevelLock(
+        $handle = $locker->withinSessionLevelLock(
             new PdoConnectionAdapter($dbConnection2),
             $lockKey,
-            function ($lockHandle): void {
-                $this->assertFalse($lockHandle->wasAcquired);
+            function () use (&$callbackExecuted): void {
+                $callbackExecuted = true;
             },
             TimeoutDuration::zero(),
         );
 
+        // THEN: Callback should not be called, handle should indicate lock was not acquired
+        $this->assertFalse($handle->wasAcquired);
+        $this->assertNull($handle->result);
+        $this->assertFalse($callbackExecuted);
         // THEN: The first connection's lock should still be intact (not decremented by a spurious release)
         $this->assertPgAdvisoryLocksCount(1);
         $this->assertPgAdvisoryLockExistsInConnection($dbConnection1, $lockKey);
     }
 
+    public function testWithinSessionLevelLockLosesCallbackResultWhenReleaseFails(): void
+    {
+        // GIVEN: A session-level lock acquired via withinSessionLevelLock callback
+        $locker = $this->initLocker();
+        $dbConnection = $this->initPostgresPdoConnection();
+        $dbConnection->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
+        $lockKey = PostgresLockKey::create('test');
+
+        // WHEN: Callback succeeds with a result but connection is killed before release
+        try {
+            $locker->withinSessionLevelLock(
+                new PdoConnectionAdapter($dbConnection),
+                $lockKey,
+                function () use ($dbConnection): string {
+                    // Kill own connection's backend to make release fail
+                    $pid = $dbConnection->pgsqlGetPid();
+                    $killerConnection = $this->initPostgresPdoConnection();
+                    $killerConnection->exec("SELECT PG_TERMINATE_BACKEND($pid)");
+
+                    return 'important-result';
+                },
+                TimeoutDuration::zero(),
+            );
+            $this->fail('Expected LockReleaseException was not thrown');
+        } catch (\Cog\DbLocker\Exception\LockReleaseException $e) {
+            // THEN: LockReleaseException is thrown (guaranteeing callback succeeded)
+            $this->assertStringContainsString('Failed to release lock', $e->getMessage());
+            // THEN: But the callback result is lost - this is a known limitation documented in ADR-003
+        } catch (\Throwable $e) {
+            $this->fail('Expected LockReleaseException but got: ' . get_class($e) . ' - ' . $e->getMessage());
+        }
+    }
+
     public function testItSanitizesCommentToPreventSqlInjection(): void
     {
         // GIVEN: A lock key with newline that could break SQL comment and inject code