[bpamiri]

Summary

A follow-up to #4330 and #4335. $advisoryLockTransactionBody() (the body of withAdvisoryLock(transaction = true)) rolled the transaction back in its outer finally when the callback had neither committed nor thrown, which means the request ended with abort. Since #4335, invokeWithTransaction() rolls back on abort itself ($invokeTransactionMethod()), so that rollback is redundant. This PR removes it, along with what only existed for it:

• the committed and threw flags (only that rollback read them);
• the outer catch (any) { threw = true; rethrow; }.

The outer try is now catch-free, so TryCatchFinallyGuardSpec (#4337) no longer needs its one exception, and this PR removes it.

Unchanged:

• the inner catch-free try/finally that releases the lock on abort or throw;
• commit-before-release on the normal path;
• the release after a failed commit;
• release-failure handling.

Why removing the rollback can't change behaviour

The removed block ran only when !committed && !threw:

• Normal path: committed is set.
• A callback or commit that throws: sets threw.
• Abort: the only case left.

On abort:

• Normal route. $runWithAdvisoryLockTransactional() reaches this method only through invokeWithTransaction(), whose transaction defaults to commit. When a Wheels transaction is already open, it takes a separate path ($runAdvisoryLockInOpenTransaction()). So this method runs inside $runInTransaction() → $transactionBody() → $invokeTransactionMethod(), whose catch-free finally rolls back on abort on every engine.

• Lucee and Adobe ColdFusion. The removed rollback ran first, and the one in $invokeTransactionMethod() right after it. Both are on the same open transaction, and the matrix in fix(model): roll back a Wheels transaction when the request ends with abort inside it #4335 showed the second as a no-op. Now only the latter runs, with the same end state.

• BoxLang. The removed rollback never ran on abort: its try had a catch clause, and BoxLang skips such a finally on abort (invariant 22). So nothing changes there.

• The migration wrapper. When a migration wraps the call (request.$wheelsTransactionWrapper), invokeWithTransaction() takes the alreadyopen route, and the body runs inside the migration's own transaction rather than through $invokeTransactionMethod(). On an abort there:

  • Lucee and Adobe ColdFusion roll back an open cftransaction when the request is aborted, which matches every abort case in the fix(model): roll back a Wheels transaction when the request ends with abort inside it #4335 matrix;
  • BoxLang skipped the removed finally anyway.

  So that route ends the same way too.

With the outer catch gone, the outer finally now also runs on BoxLang abort. It only holds the release-after-failed-commit check. On abort the inner finally has already released the lock (release.done), so it's a no-op.

Tests

No new spec. #4330's and #4335's specs are the proof:

• advisoryLockAbortReleaseSpec: abort and redirect with and without transaction = true, lock freed;
• transactionAbortRollbackSpec: writes rolled back on abort;
• advisoryLockTransactionalSpec, advisoryLockExclusionSpec, advisoryLockTenantSpec and lockingSpec.

Guard. With an empty allowlist, TryCatchFinallyGuardSpec passes on this branch. With develop's locking.cfm it fails at the old outer try (/model/locking.cfm:244), so the exception was real and is now gone.

model directory before merging develop (pass / fail / error; the six lock and transaction specs: 0 fail, skips identical to develop's database-specific ones):

Engine	SQLite	MySQL	PostgreSQL	SQL Server
BoxLang	1395 / 0 / 0	1438 / 0 / 0	1421 / 0 / 0	1450 / 0 / 0
Lucee 7	1395 / 0 / 0	1444 / 0 / 0	1421 / 0 / 0	1450 / 0 / 0
Lucee 6	1395 / 0 / 0	1444 / 0 / 0	1421 / 0 / 0	1450 / 0 / 0
Adobe ColdFusion 2023	1392 / 0 / 0	1435 / 0 / 0	1418 / 0 / 0	1447 / 0 / 0
Adobe ColdFusion 2025	1392 / 0 / 0	1435 / 0 / 0	1418 / 0 / 0	1447 / 0 / 0

After merging develop (with #4337): model directory on BoxLang, Lucee 7 and Adobe ColdFusion 2025 with MySQL (1438 / 1444 / 1435) and SQL Server (1450 / 1450 / 1447), 0 fail / 0 error. The lock and transaction specs pass 37 with 4 database-specific skips on each. The security directory, including the guard with its empty allowlist: 400 / 0 / 0.

• Lucee 7 + SQLite: full core suite 6801 passed; internal 113.
• RustCFML model directory: 1388 / 0 / 0.
• Complexity gate: passes.

🤖 Generated with Claude Code

[bpamiri]

Review @ c102d09: source looks good. Final verdict after matrix 37316988407 and PR CI

Removal is behaviour-neutral on every path

• Normal path. Commit, then release in its own guarded try, then release.done = true. Unchanged, so commit-before-release is kept.
• Throw in the callback. The inner catch-free finally releases the lock (cb.done stays false). The exception leaves the method with no outer catch now, and invokeWithTransaction()'s own catch rolls back and fires afterRollback, as before. The removed outer catch only set a flag and rethrew.
• The commit throws. The inner finally didn't release (cb.done is true), so the outer finally's "!release.done → release" safety net runs, and the exception propagates. Same as before, minus the flag.
• Abort.
  • The inner finally releases.
  • The outer finally, now catch-free and so run on BoxLang too, is a no-op because release.done is already set.
  • The rollback comes from $invokeTransactionMethod()'s catch-free finally (fix(model): roll back a Wheels transaction when the request ends with abort inside it #4335). On Lucee and Adobe that used to be the second of two rollbacks, and the fix(model): roll back a Wheels transaction when the request ends with abort inside it #4335 matrix showed the second as a clean no-op. On BoxLang the removed rollback never ran (invariant 22). Same end state on every engine.
• The migration-wrapper route (request.$wheelsTransactionWrapper, so alreadyopen): the body runs inside the migration's own transaction, not through $invokeTransactionMethod().
  • On abort, Lucee and Adobe roll back the open transaction themselves. That's the long-standing behaviour fix(model): roll back a Wheels transaction when the request ends with abort inside it #4335's matrix and its 4.1 analysis rely on.
  • BoxLang skipped the removed finally anyway.
  • So this route ends the same way.
• $runWithAdvisoryLockTransactional()'s open-transaction branch ($runAdvisoryLockInOpenTransaction()) is untouched.

The guard. TryCatchFinallyGuardSpec's one allow-list entry existed only for this outer try / catch / finally. With the catch gone the allow-list is empty, and the spec passes with no exceptions left. Per the PR body, it fails on develop's locking.cfm at that line, so the entry was real.

Docblock. The updated comments now describe the abort rollback as coming from invokeWithTransaction() / $invokeTransactionMethod(), and the outer finally as catch-free (invariant 22). That's accurate.

No findings. On the matrix I'll read advisoryLockAbortReleaseSpec (both abort cases), transactionAbortRollbackSpec, advisoryLockTransactionalSpec, advisoryLockExclusionSpec, lockingSpec and TryCatchFinallyGuardSpec on every leg, BoxLang closest, plus the PR CI tally.

[github-actions]

Wheels Test Results

222 580 tests   217 459 ✅  1h 25m 54s ⏱️
 17 312 suites    5 121 💤
     32 files          0 ❌

Results for commit c102d09.

[bpamiri]

Final: APPROVE @ c102d09 (matrix 37316988407, all 32 legs read; PR CI read)

Matrix

• 32/32 legs have 0 fail, 0 error, and every job succeeded, including rustcfml and the Compatibility Matrix Gate.

• Abort still rolls back, via fix(model): roll back a Wheels transaction when the request ends with abort inside it #4335's $invokeTransactionMethod(). transactionAbortRollbackSpec passes 5/5 on all 32 legs, including the five BoxLang legs.

• The lock is still released on abort. advisoryLockAbortReleaseSpec, with both "frees the lock after the callback aborts" cases (default path and transaction = true):

  Database	Result
  MySQL	4/4
  SQL Server	4/4
  PostgreSQL	3/4 (the same database-specific skip as before)
  Others	0/1, skipped as designed

  That's on every engine, BoxLang included.

• Lock behaviour is unchanged per database, matching fix(model): roll back a Wheels transaction when the request ends with abort inside it #4335's matrix:

  Spec	MySQL	PostgreSQL	SQL Server	Others
  advisoryLockTransactionalSpec	18/20	17/20	16/20	5/20
  advisoryLockExclusionSpec	10/12	10/12	12/12	5/12
  lockingSpec	11/11	11/11	11/11	7/11

• TryCatchFinallyGuardSpec passes 2/2 on every leg with the empty allow-list.

• Skips. The skipped specs on boxlang-mysql, adobe2023-sqlserver and lucee7-postgres are identical to fix(model): roll back a Wheels transaction when the request ends with abort inside it #4335's final matrix (37304944278), apart from fix(model): compare SQL Server date and time conditions in the column's type #4333's SQL Server temporal specs, which that older branch lacked.

PR CI: 18 pass, 3 skipping, 0 fail, 0 pending.

Behaviour is identical on the normal, throw, commit-failure, abort and migration-wrapper paths, as set out in my source review. No open findings.