Skip to content

Don't popTransaction unless it was pushed#1066

Open
MrLIVB wants to merge 1 commit into
gruelbox:masterfrom
MrLIVB:excessive-pop-transaction
Open

Don't popTransaction unless it was pushed#1066
MrLIVB wants to merge 1 commit into
gruelbox:masterfrom
MrLIVB:excessive-pop-transaction

Conversation

@MrLIVB

@MrLIVB MrLIVB commented Jul 13, 2026

Copy link
Copy Markdown

Fix #1065

Reorganizes code inside of SimpleTransactionManagerTest.withTransaction so popTransaction isn't called in finally block of connection acquiring.

@badgerwithagun badgerwithagun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @MrLIVB! Good spot. There's a small tweak needed then this is good to merge.

try {
return work.doWork(transaction);
} finally {
popTransaction();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now we need to be similarly careful about connection.setAutoCommit(autoCommit);. If pushTransaction or popTransaction throw, the connection will be left with a corrupted autocommit setting. Another nested try...finally is needed.

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.

SimpleTransactionManager.withTransaction calls popTransaction() even when pushTransaction was never reached

2 participants