From 79eb021269e76a6a62f33bcef9882c11373ab508 Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Mon, 5 Oct 2026 04:02:39 -0700 Subject: [PATCH 1/2] fix(model): roll back a Wheels transaction when the request ends with abort inside it The rollback for a failed transaction method lived in the finally/catch of a try that also has a catch clause. On BoxLang, abort skips a finally whose try has a catch clause, so a request that ended with abort inside invokeWithTransaction() (or a nested call, a savepoint unit, or withAdvisoryLock(transaction = true)) closed the transaction with the writes made before the abort. Lucee and Adobe ColdFusion rolled back. $transactionBody() now runs the method through $invokeTransactionMethod(): a catch-free try/finally around an inner try/catch that records a normal throw. When the method neither returned nor threw, the outer finally runs transaction action="rollback". Normal completion is unchanged. CLAUDE.md gains cross-engine invariant 22 for the rule. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Peter Amiri --- CLAUDE.md | 19 ++++ .../transaction-abort-rollback.fixed.md | 1 + vendor/wheels/model/transactions.cfm | 34 ++++++- .../controllers/TransactionAbortProbe.cfc | 24 +++++ .../_assets/models/TransactionAbortProbe.cfc | 50 ++++++++++ vendor/wheels/tests/routes.cfm | 4 + .../model/transactionAbortRollbackSpec.cfc | 97 +++++++++++++++++++ 7 files changed, 224 insertions(+), 5 deletions(-) create mode 100644 changelog.d/transaction-abort-rollback.fixed.md create mode 100644 vendor/wheels/tests/_assets/controllers/TransactionAbortProbe.cfc create mode 100644 vendor/wheels/tests/_assets/models/TransactionAbortProbe.cfc create mode 100644 vendor/wheels/tests/specs/model/transactionAbortRollbackSpec.cfc diff --git a/CLAUDE.md b/CLAUDE.md index 93db7f44e7..6c8c240fbf 100755 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -154,6 +154,25 @@ The framework must run on Lucee 5/6/7, Adobe CF 2018/2021/2023/2025, and BoxLang 21. **On Adobe CF, a mixin UDF that creates a subclass whose pseudo-constructor copies that same UDF onto `this` fails the first time per JVM.** Adobe CF 2023 and 2025 (Lucee and BoxLang are fine): the call's epilogue pops a super scope its prologue never pushed, so it throws `java.util.EmptyStackException` (or a `NullPointerException` on `this.SymTab_superScopes`) at `NeoPageContext.popSuperScope`. The call's own body completes normally. In Wheels, `application.wo` (a plain `wheels.Global`) runs the `global/objects.cfm` mixin `$createObjectFromRoot` to create `wheels.Public`, and Global's pseudo-constructor (`$promoteIncludedGlobalsToThis()`) copies `$createObjectFromRoot` onto the new object. That made **the first request after every cold start** a 500 at the `Public` line in `onapplicationstart.cfc` ([#3730](https://github.com/wheels-dev/wheels/issues/3730)). Any earlier request absorbs the one-time failure, including the Docker healthcheck `GET /` or a readiness retry. That is why the test suite and CI never see it, and why 51edf9dce "fixed" the wrong thing (a Public.$init include nest). Fix: create **any** Global subclass outside a mixin call first, which initializes the engine state for every Global mixin. `onapplicationstart.$init()` creates `wheels.events.SuperScopePrimer` as its first statement; keep it first, and keep the primer empty. Guard: `vendor/wheels/tests/specs/events/SuperScopePrimerSpec.cfc` (structural, because the state is once per JVM). Verify a change in this area with a **cold first request, healthcheck disabled**, never the suite. CI runs exactly that on every engine in `.github/workflows/cold-start-smoke.yml` (`tools/ci/cold-start-smoke.sh`, #3735). Deep reference: [.ai/wheels/cross-engine-compatibility.md](.ai/wheels/cross-engine-compatibility.md). +22. **On BoxLang, a request that ends with `abort` skips any `finally` whose `try` has a `catch` clause — even an unreached `catch (any) { rethrow; }`.** A catch-free `try/finally` runs, and so does one whose finally issues a `transaction action=` statement; whether the try or finally contains a transaction action makes no difference, the catch clause alone decides. Lucee 6/7 and Adobe run the finally either way, so **local Lucee and Adobe green do NOT cover this**. Keep must-run cleanup (a lock release, the rollback of a transaction on abort) in a catch-free `try/finally`; to still tell a normal throw from an abort, nest a `try/catch` that sets a flag inside it: + ```cfm + var exitState = {completed = false, threw = false}; + try { + try { + doWork(); + exitState.completed = true; + } catch (any e) { + exitState.threw = true; + rethrow; + } + } finally { + if (!exitState.completed && !exitState.threw) { + // only an abort gets here: run the must-run cleanup + } + } + ``` + Hit twice: the session-lock release in `withAdvisoryLock(transaction = true)` ([#4330](https://github.com/wheels-dev/wheels/pull/4330)), and `invokeWithTransaction()`, whose rollback lived in the finally of a try with a catch, so on BoxLang an abort inside it kept the writes made before the abort (`$invokeTransactionMethod()` in `vendor/wheels/model/transactions.cfm`). + Verify Adobe CF fixes locally before pushing — don't iterate via CI: ```bash curl -s "http://localhost:62023/wheels/core/tests?db=mysql&format=json" | \ diff --git a/changelog.d/transaction-abort-rollback.fixed.md b/changelog.d/transaction-abort-rollback.fixed.md new file mode 100644 index 0000000000..e354a6f8f6 --- /dev/null +++ b/changelog.d/transaction-abort-rollback.fixed.md @@ -0,0 +1 @@ +- A request that ends with `abort` inside a Wheels transaction (`invokeWithTransaction()`, a nested call that joins it, a savepoint unit, or `withAdvisoryLock(transaction = true)`) now rolls the transaction back on every engine, as it already did on Lucee and Adobe ColdFusion. On BoxLang the writes made before the abort were kept diff --git a/vendor/wheels/model/transactions.cfm b/vendor/wheels/model/transactions.cfm index b57b58af88..849bf7c03c 100644 --- a/vendor/wheels/model/transactions.cfm +++ b/vendor/wheels/model/transactions.cfm @@ -273,11 +273,7 @@ public void function $transactionBody(required struct ctx) { arguments.ctx.txnState.entered = true; try { - arguments.ctx.rv = $invoke( - method = arguments.ctx.method, - componentReference = this, - invokeArgs = arguments.ctx.methodArgs - ); + $invokeTransactionMethod(arguments.ctx); if ( !IsBoolean(arguments.ctx.rv) || (!IsNumeric(arguments.ctx.rv) && !arguments.ctx.rv) @@ -297,6 +293,34 @@ } } + /** + * Internal. Runs a Wheels-owned transaction's method. When the request ends with `abort` + * inside it, the method has neither returned nor thrown, and the transaction is rolled back + * here, as it is after a failure. The rollback must stay in a try/finally with no catch clause: + * on BoxLang, abort skips a finally whose try has a catch clause (cross-engine invariant 22 in + * CLAUDE.md), and the transaction would then keep the writes made before the abort. + */ + public void function $invokeTransactionMethod(required struct ctx) { + var exitState = {completed = false, threw = false}; + try { + try { + arguments.ctx.rv = $invoke( + method = arguments.ctx.method, + componentReference = this, + invokeArgs = arguments.ctx.methodArgs + ); + exitState.completed = true; + } catch (any e) { + exitState.threw = true; + rethrow; + } + } finally { + if (!exitState.completed && !exitState.threw) { + transaction action="rollback"; + } + } + } + /** * Internal. The `false` / `none` / `alreadyopen` branch of invokeWithTransaction(): run * the method with no transaction of its own. "none" still sets the open marker (so diff --git a/vendor/wheels/tests/_assets/controllers/TransactionAbortProbe.cfc b/vendor/wheels/tests/_assets/controllers/TransactionAbortProbe.cfc new file mode 100644 index 0000000000..4a41f38c9e --- /dev/null +++ b/vendor/wheels/tests/_assets/controllers/TransactionAbortProbe.cfc @@ -0,0 +1,24 @@ +/** + * Test-asset controller for transactionAbortRollbackSpec. Runs one scenario of a write inside a + * Wheels transaction (invokeWithTransaction with transaction = "commit") that ends the request with + * `abort`, or completes normally, for the row marker in params.marker. + */ +component extends="Controller" { + + function run() { + var methods = { + abort = "insertThenAbort", + nestedAbort = "outerThenNestedAbort", + savepointAbort = "outerThenSavepointAbort", + commit = "insertThenReturn", + savepointFalse = "outerThenSavepointFalse" + }; + model("transactionAbortProbe").invokeWithTransaction( + method = methods[params.scenario], + transaction = "commit", + marker = params.marker + ); + renderText("done"); + } + +} diff --git a/vendor/wheels/tests/_assets/models/TransactionAbortProbe.cfc b/vendor/wheels/tests/_assets/models/TransactionAbortProbe.cfc new file mode 100644 index 0000000000..9d4b942dde --- /dev/null +++ b/vendor/wheels/tests/_assets/models/TransactionAbortProbe.cfc @@ -0,0 +1,50 @@ +/** + * Backs transactionAbortRollbackSpec: methods run through invokeWithTransaction() that write a + * marked row and then end the request with `abort`, or return normally. Each aborting method sets + * server["wheelsTxnAbortProbe_"] right before the abort, so the spec can tell the request + * really reached the write (a positive control). The spec creates and drops the table. + */ +component extends="Model" { + + function config() { + table("c_o_r_e_txnabortprobes"); + } + + public boolean function insertThenAbort(required string marker) { + this.create(marker = arguments.marker, transaction = "none"); + server["wheelsTxnAbortProbe_" & arguments.marker] = true; + abort; + } + + public boolean function insertThenReturn(required string marker) { + this.create(marker = arguments.marker, transaction = "none"); + return true; + } + + public boolean function insertThenFalse(required string marker) { + this.create(marker = arguments.marker, transaction = "none"); + return false; + } + + // a row, then a nested invokeWithTransaction() (which joins this transaction) that aborts + public boolean function outerThenNestedAbort(required string marker) { + this.create(marker = arguments.marker & "-outer", transaction = "none"); + this.invokeWithTransaction(method = "insertThenAbort", marker = arguments.marker & "-inner"); + return true; + } + + // a row, then a savepoint unit that aborts + public boolean function outerThenSavepointAbort(required string marker) { + this.create(marker = arguments.marker & "-outer", transaction = "none"); + this.invokeWithTransaction(method = "insertThenAbort", transaction = "savepoint", marker = arguments.marker & "-inner"); + return true; + } + + // a row kept, then a savepoint unit that returns false (rolled back to the savepoint) + public boolean function outerThenSavepointFalse(required string marker) { + this.create(marker = arguments.marker & "-outer", transaction = "none"); + this.invokeWithTransaction(method = "insertThenFalse", transaction = "savepoint", marker = arguments.marker & "-inner"); + return true; + } + +} diff --git a/vendor/wheels/tests/routes.cfm b/vendor/wheels/tests/routes.cfm index 41f88cbb87..dcb7c58116 100644 --- a/vendor/wheels/tests/routes.cfm +++ b/vendor/wheels/tests/routes.cfm @@ -17,6 +17,10 @@ mapper() .post(name="browserTestLogout", pattern="/logout", to="BrowserTestSessions##destroy") .get(name="browserTestLoginAs", pattern="/login-as", to="BrowserTestLogin##create") .end() + // Abort-inside-a-transaction fixture: a write in invokeWithTransaction() whose request ends + // with abort must be rolled back. Driven by + // wheels.tests.specs.model.transactionAbortRollbackSpec. Must precede .wildcard(). + .get(name="transactionAbortProbe", pattern="/_txnabort/run", to="TransactionAbortProbe##run") .wildcard() .get(name="wheelstestbox", pattern="wheels/core/tests", to="wheels##public##tests") .get(name="sampleLinkToTest", pattern="sample/linktotest", to="sample##linktotest") diff --git a/vendor/wheels/tests/specs/model/transactionAbortRollbackSpec.cfc b/vendor/wheels/tests/specs/model/transactionAbortRollbackSpec.cfc new file mode 100644 index 0000000000..78f7e62ea9 --- /dev/null +++ b/vendor/wheels/tests/specs/model/transactionAbortRollbackSpec.cfc @@ -0,0 +1,97 @@ +/** + * A request that ends with `abort` inside a Wheels transaction (invokeWithTransaction(), a nested + * call that joins it, or a savepoint unit) leaves none of the transaction's writes behind, and a + * transaction that completes normally is unaffected. Each scenario runs as its own HTTP request + * through $testClient (TransactionAbortProbe controller, route in vendor/wheels/tests/routes.cfm), + * since an abort ends the request it runs in. Each aborting scenario also checks a marker set right + * before the abort, so it can't pass because the request never reached the write. + */ +component extends="wheels.WheelsTest" { + + function beforeAll() { + variables.g = application.wo; + variables.migration = CreateObject("component", "wheels.migrator.Migration").init(); + try { + variables.migration.dropTable("c_o_r_e_txnabortprobes"); + } catch (any e) { + } + var t = variables.migration.createTable(name = "c_o_r_e_txnabortprobes"); + t.string(columnNames = "marker"); + t.create(); + StructDelete(application.wheels.models, "TransactionAbortProbe"); + } + + function afterAll() { + try { + variables.migration.dropTable("c_o_r_e_txnabortprobes"); + } catch (any e) { + } + StructDelete(application.wheels.models, "TransactionAbortProbe"); + } + + function newMarker(required string scenario) { + return arguments.scenario & Left(Replace(CreateUUID(), "-", "", "all"), 10); + } + + // Runs a scenario in its own request; returns its status code. + function runScenario(required string scenario, required string marker) { + var tc = $testClient(); + tc.get("/_txnabort/run?scenario=#arguments.scenario#&marker=#arguments.marker#"); + return tc.statusCode(); + } + + function rowsFor(required string marker) { + return variables.g.model("transactionAbortProbe").count(where = "marker LIKE '#arguments.marker#%'"); + } + + function reachedAbort(required string marker) { + return StructKeyExists(server, "wheelsTxnAbortProbe_" & arguments.marker); + } + + function run() { + + describe("A request that ends with abort inside a Wheels transaction", () => { + + it("leaves none of the transaction's writes", () => { + var marker = newMarker("abort"); + expect(runScenario("abort", marker)).toBe(200); + expect(reachedAbort(marker)).toBeTrue("the request never reached the write before the abort"); + expect(rowsFor(marker)).toBe(0); + }); + + it("leaves none of them when the abort is in a nested call that joins the transaction", () => { + var marker = newMarker("nested"); + expect(runScenario("nestedAbort", marker)).toBe(200); + expect(reachedAbort(marker & "-inner")).toBeTrue("the request never reached the write before the abort"); + expect(rowsFor(marker)).toBe(0); + }); + + it("leaves none of them when the abort is in a savepoint unit", () => { + var marker = newMarker("savepoint"); + expect(runScenario("savepointAbort", marker)).toBe(200); + expect(reachedAbort(marker & "-inner")).toBeTrue("the request never reached the write before the abort"); + expect(rowsFor(marker)).toBe(0); + }); + + }); + + describe("A Wheels transaction that completes normally", () => { + + it("keeps its writes", () => { + var marker = newMarker("commit"); + expect(runScenario("commit", marker)).toBe(200); + expect(rowsFor(marker)).toBe(1); + }); + + it("keeps the outer write when a savepoint unit returns false", () => { + var marker = newMarker("savepointFalse"); + expect(runScenario("savepointFalse", marker)).toBe(200); + expect(rowsFor(marker & "-outer")).toBe(1); + expect(rowsFor(marker & "-inner")).toBe(0); + }); + + }); + + } + +} From b0645e0c9669cec1c3520ce32a16854ea7485b8e Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Mon, 5 Oct 2026 04:03:27 -0700 Subject: [PATCH 2/2] docs(model): name the catch clause as what makes BoxLang skip a finally on abort The release comments in $advisoryLockTransactionBody() said a transaction-action statement in a finally makes BoxLang skip it on abort. Measured: the trigger is a catch clause on the try; transaction actions make no difference. The comments now say so and point at cross-engine invariant 22, and note that invokeWithTransaction() performs the abort rollback on BoxLang. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Peter Amiri --- vendor/wheels/model/locking.cfm | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/vendor/wheels/model/locking.cfm b/vendor/wheels/model/locking.cfm index 034401a259..72ba74712d 100644 --- a/vendor/wheels/model/locking.cfm +++ b/vendor/wheels/model/locking.cfm @@ -250,10 +250,11 @@ } cb.done = true; } finally { - // This inner finally holds the lock RELEASE and contains NO transaction-action statement, - // so BoxLang runs it even when the callback ends the request with `abort`: a `transaction - // action="…"` inside a finally is skipped by BoxLang when an abort leaves the surrounding - // cftransaction (#4219 — Lucee/Adobe run it either way). On an abnormal exit (abort or a + // This inner finally holds the lock RELEASE, and its try has NO catch clause, so BoxLang + // runs it even when the callback ends the request with `abort`. On BoxLang, abort skips a + // finally whose try has a catch clause, even an unreached catch (any) { rethrow; }; + // transaction actions make no difference (cross-engine invariant 22; #4219). Lucee/Adobe + // run it either way. Keep this inner try CATCH-FREE. On an abnormal exit (abort or a // throw -> cb.done stays false) the lock is released HERE; on the normal path cb.done is // true and the commit-then-release below runs instead, preserving commit-before-release so // the next holder reads committed state. Unscoped struct writes persist past the catch on @@ -287,11 +288,11 @@ rethrow; } finally { // Roll back ONLY on the ABORT path — not committed, and not a caught throw. An aborting - // callback is never caught, so invokeWithTransaction's catch never runs and the engine would - // otherwise decide the open transaction's fate at block exit. This rollback is a - // transaction-action, and BoxLang skips a finally that contains one when an abort leaves the - // cftransaction; the LOCK release above sits in a separate finally with no transaction-action, - // so it still runs on every engine on abort. Lucee/Adobe run this finally normally. + // callback is never caught, so invokeWithTransaction's catch never runs. This outer try has a + // catch clause, so BoxLang skips this finally on abort (cross-engine invariant 22); there the + // rollback comes from $invokeTransactionMethod(), whose catch-free finally runs this method + // through invokeWithTransaction(), and the LOCK release above sits in a separate catch-free + // try/finally. Lucee/Adobe run this finally normally. if (!local.committed.flag && !threw.flag) { try { transaction action="rollback";