Skip to content

fix(txn): keep a connection usable after a refused BEGIN - #1375

Draft
zahariash wants to merge 2 commits into
LadybugDB:mainfrom
zahariash:fix/refused-begin-mode
Draft

zahariash wants to merge 2 commits into
LadybugDB:mainfrom
zahariash:fix/refused-begin-mode

Conversation

@zahariash

Copy link
Copy Markdown
Contributor

This work was produced with the help of language models.

Fixes #1316.

While another connection held a write transaction, BEGIN TRANSACTION was refused with Cannot start a new write transaction in the system. Only one write transaction at a time is allowed in the system., but the connection had already been switched to manual mode. Left in manual mode with no transaction, its next statement, even MATCH (n:t) RETURN count(*), segfaulted the process, and ROLLBACK could not recover it.

Fix. The read, write and recovery begins now share beginManualTransaction, which switches to manual mode only after the transaction manager has started the transaction. A refused BEGIN leaves the connection in auto-commit mode, so its next statement runs normally and a write gets the usual single-writer error. The header states that MANUAL implies an active transaction. No ABI change: one private non-virtual method.

Tests. RefusedBeginKeepsConnectionUsable in ddl_serial_execution.test runs ROLLBACK, a read, a refused write and, once the writer commits, a full transaction on the refused connection; it trips a DASSERT on main. Unit tests cover refused write, recovery and prepared begins, a refused BEGIN in a multi-statement query, and a poisoned WAL; all fail on main. Full ctest: 3186/3186.

BEGIN TRANSACTION switched the connection to manual mode before the
transaction manager could refuse the begin (only one write transaction at
a time). The refused connection was then in manual mode without a
transaction, and its next statement bound with a null transaction and
segfaulted. The mode now changes only after the begin succeeds; read,
write and recovery begins share that order.

The header pins that MANUAL implies an active transaction. Tests cover a
refused write and recovery begin, a refused prepared BEGIN, a refused BEGIN
in a multi-statement query, a BEGIN on a poisoned WAL, and the connection's
next statements, including ROLLBACK, after a refused BEGIN.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@adsharma

Copy link
Copy Markdown
Contributor

Finding: header comment "Callers hold ClientContext's mutex" is wrong — method takes TransactionContext::mtx itself,
commit/rollback take none; rests on single-thread-per-conn. Reword, no behavior change.

Only beginManualTransaction takes the context's mutex; commit, rollback and
clearTransaction take none, so the invariant rests on one thread per
connection rather than on a caller-held lock.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zahariash

Copy link
Copy Markdown
Contributor Author

Thanks, reworded in 4d3b8ed: only beginManualTransaction takes mtx, and the invariant relies on a connection being used by one thread at a time. No behavior change.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: a connection whose transaction begin was refused segfaults on its next statement

2 participants