Skip to content

Commit 0b7622e

Browse files
author
wshallwshall
committed
test(store): widen the writer-txn guard to the nested-transaction verbs
tests/test_writer_txn_is_the_only_begin.py pins which functions may open a transaction, but its AST scan matched BEGIN only. SAVEPOINT opens a nested transaction and ROLLBACK TO unwinds one, so both landed in exactly the gap the guard exists to close. RELEASE is scanned with them for the mirror hazard: it commits a savepoint and every savepoint opened after it, so a stray or mispaired one makes durable what the caller still expected to be able to roll back. Widening the verb list alone would not have worked. The matcher was gated on ast.Constant upstream of the prefix check, and a savepoint name has to be interpolated, so real savepoint code is f-string or concatenation code. Measured: the old scan run over PR 1233's store.py -- the live user of these verbs -- finds its two known BEGINs and reports the file clean. The scan now reconstructs f-strings and "+" concatenation, in the shape of tests/test_adr0157_fence_scope.py. executescript is now scanned as well, statement by statement, resolving a script passed by module-constant name. It was the one call carrying many statements in a single argument, and nothing read it. COMMIT and bare ROLLBACK stay out of the verb set on purpose: MessageStore._read issues one of each as the ordinary whole-transaction close, and matching either would red a clean tree on the statements that keep it clean. Control arms pin both exclusions, alongside executemany and a SELECT whose text merely names a savepoint. _ALLOWED is left byte-identical because PR 1227 has live edits on its MessageStore._read entry. The new per-verb counts live in a separate table joined at check time, with the merge condition and the value-shape constraint recorded so the split does not outlive its reason. _ALLOWED_NESTED ships empty, which is the measurement rather than an unfinished table: store.py holds no savepoint statement today. Whichever of this change and PR 1233 lands second registers that PR's four sites, which the failure message names by verb, function and line. Receipts are now four and include a pinned count of arguments the scan could not read as text, since a new unreadable argument is the shape an evasion takes.
1 parent ec32c96 commit 0b7622e

3 files changed

Lines changed: 391 additions & 52 deletions

docs/adr/0159-cancellation-safe-pooled-connection-release-mid-txn-discard-at-the-acquire-chokepoint.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,13 @@ the next reader does not re-derive the wrong precedent from the same comment.
204204
`_read`'s pooled read snapshot, which runs on a borrowed connection and already unwinds in its own
205205
`except BaseException: ROLLBACK`. So **no writer opens a transaction on `self._db` outside
206206
`_writer_txn`**, and an eighteenth that tried would fail the build rather than quietly reopen the
207-
hole.
207+
hole. The same scan covers the nested-transaction verbs. `SAVEPOINT` and `ROLLBACK TO` open and
208+
unwind a nested transaction, so they carry this ADR's shape exactly. `RELEASE` is scanned for the
209+
mirror-image hazard rather than the same one: it commits a savepoint and every savepoint opened
210+
after it, so a stray or mispaired one does not leak an open transaction, it makes durable what the
211+
caller still expected to be able to roll back. `store.py` holds none of the three today, so their
212+
carve-out table is empty and the first one added has to register itself with a count and a reason
213+
rather than arrive as grounds for deleting the check.
208214

209215
**What remains is the SHORT writers: a different property, the same mechanism.** A short writer
210216
takes `self._lock`, issues its DML with no `BEGIN` of its own, and calls `_commit()`. The paragraph

tests/test_tooling_partition.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@
139139
# .github/**, the ledger) is not tripped by one, so gating this behind scripts/** would
140140
# leave exactly the change that reintroduces the defect facing nothing.
141141
"test_username_access_key_screen.py",
142-
# AST-scans messagefoundry/store/store.py and reds on any `execute("BEGIN")` outside
142+
# AST-scans messagefoundry/store/store.py and reds on any `execute("BEGIN")` -- or on the
143+
# nested-transaction verbs `SAVEPOINT`, `ROLLBACK TO` and `RELEASE` -- outside
143144
# `_writer_txn` (ADR 0159). Same shape as sqlserver_encrypt_pass_tables below -- a guard whose
144145
# subject is a store module it reads rather than imports -- and it stays here for this file's
145146
# standard gating reason, checked against the gate rather than assumed: what it catches is an

0 commit comments

Comments
 (0)