Fix database lease ownership, transaction cleanup and read extensions

[binaryfire]

This fixes the database lifecycle issues raised in the review of #653 and removes unused stream-ID plumbing from response cancellation. Ordinary queries still benefit from early connection release and reuse. The changes close gaps in ownership, transaction cleanup, and read-driver extensions.

End logical connection ownership

A retained connection or query builder could borrow another pool slot after its owning coroutine had finished. The cleanup registered for that owner had already run, so nothing returned the new slot to the pool.

Logical leases now end when their coroutine or task releases or discards them. A connection whose initial setup fails is also ended, so a listener retaining it cannot later borrow a slot without registered cleanup. Attempting to borrow through an ended lease throws an exception explaining where to resolve the connection. This check runs only before a new borrow. Early release, reconnect, and recovery from a failed reacquisition still work within the owning execution. Release and rollback callbacks can still use the session while cleanup owns it.

Keep raw transactions on their sessions

Transactions started through PDO or SQL don't update the framework's transaction counter. Early release previously treated those sessions as idle, allowing another request to use a transaction it didn't start.

Pinning now checks the physical transaction state of both open PDO handles. This covers native and SQL-started transactions on the writer or reader without opening an unused handle. For example, the transaction below retains its session across the HTTP call:

$connection = DB::connection();
$pdo = $connection->getPdo();
$pdo->beginTransaction();

try {
    Http::post($url, $payload);
    $pdo->commit();
} catch (Throwable $exception) {
    $pdo->rollBack();
    throw $exception;
}

The public inTransaction() method keeps its existing writer-only behavior. Code using temporary tables, session locks, or other session-dependent operations still uses withPinnedSession() as documented.

Settle unfinished transactions before reuse

Pool release now checks physical transaction state after callbacks finish and the logical lease detaches. This also catches a raw transaction opened by a release callback. If a transaction remains, or its state can't be inspected, the slot is discarded immediately so it doesn't sit idle holding transaction locks.

The discard decision is made before logging, so a failing logger can't return a dirty session to the pool. Cleanup still settles the slot when cancellation occurs. Other preparation failures retain their existing reconnect-before-reuse behavior when no transaction remains.

PDO disconnect now cleans both distinct handles and attempts reader cleanup even if writer cleanup fails. Cancellation retains precedence over ordinary errors. Shared in-memory SQLite keeps its committed data and schema after successful rollback and replacement of the pooled wrapper.

Honor effective read-driver extensions

An explicit ::read pool could select a DB::extend driver from a read record, then incorrectly try to rebuild its result as a PDO lease. That bypassed the extension and could fail with an unsupported-driver error or a type error.

Lease eligibility now checks every effective read record, including a single record and a list of records. These extensions retain their complete connection object. Named and top-level extension precedence, write-side construction, and database identity checks remain unchanged. Classification happens once when the pool is created.

Remove unused response stream IDs

Response cancellation tracks active producers by connection and coroutine. The stored stream ID wasn't used. This removes it and its argument forwarding from the response bridge, HTTP, gRPC, WebSocket, and test-fixture paths.

Connection-close cancellation still stops all opted-in producers on that connection, preserves other connections, and removes registrations after production. The existing HTTP/2 follow-up now states that stream identity will be added when a released Swoole stream-cancel event needs it.

Verification

The database regressions fail against the original source and pass with these changes. Coverage includes expired owners, raw transactions, read extensions, callback failures, cancellation, and cleanup of both PDO handles. Existing response tests verify that removing stream IDs preserves cancellation behavior.

Formatting, PHPStan, the full components suite, Testbench, and package-consumer checks passed. The MySQL, MariaDB, and PostgreSQL integration runs also passed, including their package-specific suites.

There are no new ownership checks on ordinary query execution or new database round trips for transaction inspection. The additional checks run at pool creation, acquisition, and release boundaries.

Summary by CodeRabbit

• Bug Fixes
  • Prevented pooled database connections from being reused while physical transactions remain open, including transactions started through PDO or SQL.
  • Improved cleanup of unfinished transactions across read and write connections, while preserving shared in-memory SQLite connections.
  • Prevented ended or discarded connection leases from borrowing another session.
  • Kept disconnect cancellation scoped to the affected connection.

Note

Fix database lease ownership, transaction cleanup, and pooled connection discard

• Adds an ended flag to ConnectionLease so PDO resolution after release or discard throws LogicException instead of borrowing a new pooled session; idle release no longer ends the lease
• PooledConnection.release now inspects for open physical transactions and discards the connection (with rollback) instead of reusing it; inspection failures also trigger discard
• Connection.hasPinnedSession and the new hasPhysicalTransaction keep the session pinned during raw PDO/SQL transactions, including on read connections
• DatabasePool.supportsSessionLeases disables session leases for configured extensions, including single-record and listed read-driver configs
• PdoConnection.disconnectDriverResources rolls back transactions on both distinct write and read PDOs before forgetting them
• Removes the stream ID parameter from ResponseBridge::send and ResponseCancellation.register across HTTP, WebSocket, and gRPC servers; registration now uses only the connection and producer coroutine identifiers
• Risk: callers of ResponseBridge::send() that pass a stream ID (now removed) will break; extensions configured on read endpoints now change session-lease eligibility in DatabasePool.php

^{Macroscope summarized a3d1479.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: Repository: hypervel/components-backup/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: ec349596-8d38-4e2a-87c9-4e2f118be55d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Fix database lease ownership, transaction cleanup, and read extensions

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Prevent ended database leases from borrowing unowned pool slots while preserving early release.
• Pin raw transactions and discard sessions with unfinished or uninspectable transactions.
• Preserve effective read-driver extensions and remove unused response stream-ID plumbing.

Diagram

graph TD
  C["Owning execution"] --> L["Logical lease"] --> P["Database pool"] --> S["Pooled slot"] --> H["PDO handles"]
  R["Read configuration"] --> P
  P -->|"extension selected"| X["Whole connection"]

Loading

High-Level Assessment

Keep ownership checks at borrow boundaries and transaction inspection at release boundaries. Disabling leases broadly would sacrifice safe early reuse, while per-query ownership checks or SQL parsing would add complexity without addressing cleanup as directly.

Files changed (19) +590 / -82

Bug fix (5) +97 / -22

Connection.phpInclude physical transactions in session pinning +12/-1

Include physical transactions in session pinning

• Adds an internal physical-transaction check and uses it when deciding whether a connection's session must remain pinned.

src/database/src/Connection.php

PdoConnection.phpInspect and clean both open PDO handles +33/-8

Inspect and clean both open PDO handles

• Detects transactions on open writer and reader handles without resolving lazy handles. Disconnect attempts rollback on both distinct handles, retains failure precedence, and clears driver resources.

src/database/src/PdoConnection.php

ConnectionLease.phpEnd logical leases after owner cleanup +13/-4

End logical leases after owner cleanup

• Makes terminal release and discard prevent later borrows by retained connections or builders. Early idle release and failed-acquisition recovery remain non-terminal.

src/database/src/Pool/ConnectionLease.php

DatabasePool.phpClassify effective read extensions before enabling leases +15/-6

Classify effective read extensions before enabling leases

• Checks single and listed effective read records for extensions and preserves endpoint identity checks. Pools requiring extension-created connections retain whole-connection ownership.

src/database/src/Pool/DatabasePool.php

PooledConnection.phpDiscard slots with unfinished physical transactions +24/-3

Discard slots with unfinished physical transactions

• Inspects transaction state after release callbacks and lease detachment, discarding dirty or uninspectable slots before they can become idle. Settlement continues despite logging failures or cancellation.

src/database/src/Pool/PooledConnection.php

Refactor (5) +6 / -9

Server.phpStop forwarding unused gRPC response stream IDs +2/-2

Stop forwarding unused gRPC response stream IDs

• Removes the unused stream-ID argument from normal and error response emission.

src/grpc/src/Server/Server.php

ResponseBridge.phpRemove stream IDs from response registration +1/-2

Remove stream IDs from response registration

• Drops the unused send argument and registers cancellable producers by connection without forwarding a stream ID.

src/http-server/src/ResponseBridge.php

Server.phpSimplify HTTP response bridge calls +0/-1

Simplify HTTP response bridge calls

• Stops passing the unused request stream ID during response emission.

src/http-server/src/Server.php

ResponseCancellation.phpRemove unused stream identity from cancellation state +2/-3

Remove unused stream identity from cancellation state

• Keeps active-producer registration keyed by connection and coroutine while removing the stored stream ID and registration argument.

src/server/src/ResponseCancellation.php

Server.phpSimplify WebSocket handshake response emission +1/-1

Simplify WebSocket handshake response emission

• Removes unused stream-ID forwarding when sending the HTTP handshake response.

src/websocket-server/src/Server.php

Tests (6) +340 / -45

DatabaseConnectionLeaseLifecycleTest.phpCover ended owners and raw-transaction cleanup +82/-2

Cover ended owners and raw-transaction cleanup

• Verifies retained builders cannot borrow after coroutine or task cleanup, including discard paths. Confirms raw transactions opened by application code or release listeners are rolled back without losing committed shared-memory SQLite data.

tests/Database/DatabaseConnectionLeaseLifecycleTest.php

DatabaseConnectionLeaseTest.phpTest raw-transaction pinning and read extensions +69/-21

Test raw-transaction pinning and read extensions

• Adds native, SQL-started, and read-PDO transaction pinning cases across manual and HTTP-triggered release. Covers named and single or listed read-record extensions retaining their connection objects.

tests/Database/DatabaseConnectionLeaseTest.php

DatabasePdoConnectionTest.phpExercise distinct-handle disconnect cleanup +69/-6

Exercise distinct-handle disconnect cleanup

• Verifies both PDO handles receive cleanup, duplicate and lazy handles are handled appropriately, and reader cancellation takes precedence over an earlier ordinary rollback failure.

tests/Database/DatabasePdoConnectionTest.php

disconnect-server.phpUpdate disconnect fixture for the response bridge signature +1/-1

Update disconnect fixture for the response bridge signature

• Removes unused stream-ID forwarding from the disconnect integration fixture.

tests/HttpServer/Fixtures/disconnect-server.php

ResponseBridgeTest.phpPreserve connection-scoped cancellation coverage +11/-12

Preserve connection-scoped cancellation coverage

• Removes redundant stream-ID test cases while checking that disconnect cancels opted-in producers on one connection and leaves another connection unaffected.

tests/HttpServer/ResponseBridgeTest.php

PooledConnectionTest.phpVerify dirty-slot discard and failure settlement +108/-3

Verify dirty-slot discard and failure settlement

• Tests raw PDO rollback and discard, transactions opened during release callbacks, and settlement despite logging, inspection, or cancellation failures.

tests/Integration/Database/PooledConnectionTest.php

Documentation (3) +147 / -6

2026-10-08-1451-framework-lease-cleanup.mdRecord the lease-cleanup design and verification plan +141/-0

Record the lease-cleanup design and verification plan

• Documents the reported lifecycle defects, intended ownership boundaries, transaction settlement rules, read-extension classification, response cleanup, and regression coverage.

docs/plans/2026-10-08-1451-framework-lease-cleanup.md

todo.mdClarify future HTTP/2 stream-cancel integration +1/-1

Clarify future HTTP/2 stream-cancel integration

• Specifies that response registrations should gain stream identity when a supported Swoole stream-cancel event becomes available.

docs/todo.md

database.mdDocument lease lifetime and raw-transaction behavior +5/-5

Document lease lifetime and raw-transaction behavior

• Explains ownership expiry, automatic pinning and cleanup of PDO- or SQL-started transactions, and read-record extension handling. Retains guidance to pin other session-dependent operations explicitly.

src/docs/database.md

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[greptile-apps]

Confidence Score: 5/5

[Critical risk] Fixes database connection lease lifecycle and transaction cleanup.

The PR appears safe to merge; the previous failed-owner finding is fixed and no new blocking issues were found.

What we checked:

• Failed owners cannot borrow again: ConnectionResolver::connection() discards the logical owner. Its ended flag blocks another borrow after the session detaches.
• Discard frees pool capacity: ConnectionPool::destroyConnection() removes the slot from both pool records in finally, even when close throws.

Summary

This PR ends database lease ownership, protects raw transactions during early release, cleans both PDO handles, preserves effective read-driver extensions, and removes unused response stream IDs.

• The latest changes close the failed-initial-owner gap from the previous review.
• Failed reacquisition remains recoverable within a live owner.
• Added assertions cover retained connections after setup failure and cancellation.
• No new actionable issues were found.

_{Reviews (2) · Last reviewed commit: "Clarify failed initial ownership in the ..." · Reviewed by Greptile}