Skip to content

Pass captured transaction to ExecuteOnConnection callback instead of re-reading field#498

Draft
Copilot wants to merge 2 commits intofix-transaction-sharingfrom
copilot/sub-pr-497
Draft

Pass captured transaction to ExecuteOnConnection callback instead of re-reading field#498
Copilot wants to merge 2 commits intofix-transaction-sharingfrom
copilot/sub-pr-497

Conversation

Copy link
Contributor

Copilot AI commented Mar 8, 2026

Callers like WriteImageFile and PostUpdateProcessing were re-reading this.transaction inside their lambdas without holding transactionLock. When no active transaction exists, the lambda falls through to the unlocked path at line 90 — a concurrent BeginTransaction between the check and the callback could cause the lambda to execute against a mismatched connection/transaction pair.

Changes

  • ExecuteOnConnection signature: Func<SqliteConnection, T>Func<SqliteConnection, SqliteTransaction?, T>
  • Inside lock: passes lock-captured currentTransaction directly to callback — func(currentTransaction.Connection, currentTransaction)
  • No-transaction path: passes nullfunc(connection, null)
  • Transactional callers (WriteImageFile, PostUpdateProcessing): updated to use the provided tx parameter instead of this.transaction
  • Non-transactional callers: use (connection, _) discarding the parameter
// Before — re-reads this.transaction outside the lock
ExecuteOnConnection(connection => connection.Execute(sql, param, this.transaction));

// After — uses the lock-captured reference passed by ExecuteOnConnection
ExecuteOnConnection((connection, tx) => connection.Execute(sql, param, tx));

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…OnConnection callback

Co-authored-by: jamesmoore <6506748+jamesmoore@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix locking mechanism for transaction closing based on feedback Pass captured transaction to ExecuteOnConnection callback instead of re-reading field Mar 8, 2026
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.

2 participants