Skip to content

fix(session): await the rollback in BLiteSession; add RollbackAsync (#307) - #316

Open
CaffeinatedCoder wants to merge 2 commits into
EntglDb:mainfrom
CaffeinatedCoder:fix/307-session-rollback-async
Open

CaffeinatedCoder wants to merge 2 commits into
EntglDb:mainfrom
CaffeinatedCoder:fix/307-session-rollback-async

Conversation

@CaffeinatedCoder

Copy link
Copy Markdown
Collaborator

What was wrong

BLiteSession.Rollback() started CurrentTransaction.RollbackAsync() and returned without awaiting the ValueTask (#307, BLiteSession.cs:107-116). The session forgot the transaction at once, but the rollback finished later: the abort record was written to the WAL, then the transaction became Aborted and its rollback handlers restored the collection's free-space index. Any exception from that work was lost. The README's session sample uses this call in its catch block.

Measured with the new test (300 rollbacks while another task commits to a second collection): 236–244 of 300 transactions were still Active when Rollback() had returned.

Fix

  • BLiteSession.RollbackAsync() (new) awaits the rollback. It forgets the transaction before the rollback starts, so a second Rollback/RollbackAsync or a CommitAsync on the same session cannot act on a transaction that is already rolling back. It takes no CancellationToken, like ITransaction.RollbackAsync(): a rollback is not something to abandon halfway.
  • Rollback() blocks on it (GetAwaiter().GetResult()), the pattern Transaction.Dispose already uses for the same call. Its XML doc now says so and points at RollbackAsync.
  • The generated typed session wrapper (OpenTypedSession) gets RollbackAsync() next to Rollback().
  • README session sample: await session.RollbackAsync();.

Not changed: BLiteEngine.Rollback() stays the documented no-op. Removing both synchronous methods is for v6 (#296). The blocking Rollback() is one more synchronous wait of the kind #290 lists for the browser runtime; RollbackAsync is the one to use there.

Tests

BLiteSessionTests, five new tests:

Test upstream/main this PR
RollbackAsync_TransactionIsAborted_WhenItReturns (300 rollbacks under a concurrent committer) fails, 300 of 300 still active (with a fire-and-forget shim for the new method) passes
Rollback_TransactionIsAborted_WhenItReturns fails, 236–244 of 300 still active in three runs passes
RollbackAsync_DiscardsTheWrites_AndClearsCurrentTransaction passes passes
RollbackAsync_WithoutTransaction_IsANoOp passes passes
RollbackAsync_CalledTwice_RollsBackOnce passes passes

A test that a failing WAL abort write surfaces to the caller needs a fault-injection seam the storage engine does not have; Transaction.RollbackAsync already propagates the exception, and RollbackAsync is a plain forward of its ValueTask.

Full suite dotnet test tests/BLite.Tests (macOS, net10.0): 3097 passed, 0 failed, 4 skipped.

Fixes #307

🤖 Generated with Claude Code

…ntglDb#307)

BLiteSession.Rollback() started CurrentTransaction.RollbackAsync() and
returned without awaiting it, so the WAL abort record, the Aborted state and
the rollback handlers (free-space-index restore) ran later, and a failure of
that work was never observed. Under concurrent commits 236-244 of 300
transactions were still Active when Rollback() returned.

BLiteSession.RollbackAsync() awaits the rollback and forgets the transaction
before it starts, so a second call cannot roll the same transaction back
twice. Rollback() now blocks on it, the pattern Transaction.Dispose already
uses. The generated typed session wrapper gets RollbackAsync() too, and the
README session sample awaits it.

Tests: BLiteSessionTests, five tests; the two load tests fail on
upstream/main (244 and 300 of 300 still active) and pass with the fix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@mrdevrobot mrdevrobot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. Verified: Rollback() now completes before it returns; the load test fails on 0450b97 (264 of 300 transactions still Active) and passes here. The new RollbackAsync() is accepted for consistency with CommitAsync, but give it a CancellationToken parameter (default) on BLiteSession and on the generated session classes, as the other async entry points have. Then this can be merged after verification.

@CaffeinatedCoder
CaffeinatedCoder force-pushed the fix/307-session-rollback-async branch from f050b39 to 29df17c Compare October 19, 2026 04:21
BLiteSession.RollbackAsync and the generated session wrapper's RollbackAsync
take a CancellationToken, like CommitAsync. The token is checked before the
rollback starts; a cancelled token throws and leaves the transaction active.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CaffeinatedCoder
CaffeinatedCoder force-pushed the fix/307-session-rollback-async branch from 29df17c to 22d66a1 Compare October 19, 2026 04:21
@CaffeinatedCoder

Copy link
Copy Markdown
Collaborator Author

Done in 22d66a1: BLiteSession.RollbackAsync and the generated session's RollbackAsync now take CancellationToken ct = default, like CommitAsync; the token is checked before the rollback starts, so a cancelled token throws and leaves the transaction active (new test RollbackAsync_CancelledToken_Throws_AndLeavesTheTransactionActive).

This branch was successfully deployed

1 active deployment
Production — 22d66a1c Deployed Oct 19, 2026 by CaffeinatedCoder via build_and_publish #795
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working package/core Affects BLite.Core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BLiteSession.Rollback() does not await the rollback: it returns while the transaction is still active, and failures go unobserved

2 participants