Skip to content

fix(core): accept transaction callbacks without Sync - #4518

Open
shuvroroy wants to merge 2 commits into
transact-rs:mainfrom
shuvroroy:fix/4513-transaction-callback-sync
Open

shuvroroy wants to merge 2 commits into
transact-rs:mainfrom
shuvroroy:fix/4513-transaction-callback-sync

Conversation

@shuvroroy

@shuvroroy shuvroroy commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Connection::transaction rejects callbacks that capture Send but non-Sync values such as std::sync::mpsc::Receiver. The callback is moved into the returned future and invoked once, so this removes the unnecessary Sync bound while retaining Send.

Add a shared regression helper to the native PostgreSQL, MySQL/MariaDB, SQLite, and Any suites. It holds a receiver across an await, checks that the transaction future is Send, and verifies commit on Ok and rollback on Err.

Does your PR solve an issue?

Fixes #4513.

Is this a breaking change?

Yes, for custom Connection implementations that override transaction with the old F: Sync bound: they must relax that bound to match the trait. Existing callers remain compatible. All in-tree drivers inherit the default implementation.

Validation

  • The shared regression fails to compile with the original Sync bound and compiles after its removal.
  • All eight native/Any regression runs passed: PostgreSQL 17.11, MySQL 8.4.13, MariaDB 11.4.13, and SQLite.
  • SQLite integration suite: 45 passed, one existing ignored test.
  • Core unit tests with --all-features: 11 passed.
  • All-driver library Clippy with -D warnings, cargo check -p sqlx-test, formatting, and git diff --check passed.

Require only Send for the owned FnOnce callback. The returned future
remains Send, while captures such as mpsc::Receiver are now accepted.
Share regression coverage between PostgreSQL, MySQL/MariaDB, SQLite,
and Any connections. Check non-Sync captures, future sendability,
commit, and rollback.
@shuvroroy
shuvroroy force-pushed the fix/4513-transaction-callback-sync branch from 3e337ad to eea4454 Compare November 10, 2026 13:51
@shuvroroy
shuvroroy marked this pull request as ready for review November 10, 2026 16:34
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.

Connection::transaction requires the callback to be Sync, but only Send is needed

1 participant