Skip to content

fix(sqlite): await cleanup after startup failures - #4494

Open
samjingwen wants to merge 1 commit into
transact-rs:mainfrom
samjingwen:fix/sqlite-startup-cleanup
Open

samjingwen wants to merge 1 commit into
transact-rs:mainfrom
samjingwen:fix/sqlite-startup-cleanup

Conversation

@samjingwen

Copy link
Copy Markdown

Does your PR solve an issue?

No existing issue is linked.
This fixes SQLite connection cleanup after startup errors in SQLx 0.9.0/current main.

SqliteConnectOptions::connect() establishes a worker before executing startup PRAGMAs and registering collations.
If either step fails, dropping the connection does not wait for the worker to release its SQLite handle.
The caller can therefore receive an initialization error while the database connection is still being cleaned up.

Changes

  • Extract the existing worker-shutdown sequence into a crate-private helper, also used by normal close().
  • Await that helper on post-establishment initialization errors, preserving the original error.
  • Skip optional optimize-on-close commands on this failure path, since optimization can itself fail before shutdown.
  • Validate collation names before converting the callback Arc into a raw pointer, avoiding a leaked reference when name validation fails.

The regression calls the real SqliteConnection::connect_with() API for invalid PRAGMAs, a corrupt database, and partially registered collations followed by an invalid name.
It pauses the worker at an existing tracing boundary, verifies that error return remains pending while the worker still owns the connection, then checks a native SQLite destructor acknowledgement before accepting the returned error.
It also checks original-error preservation, unchanged corrupt-file contents, and release of both registered and rejected collation callbacks.
The test has its own binary because SQLite auto-extension registration is process-global, and unregisters only its own callback.

Is this a breaking change?

No public API or successful-connection behavior changes.
Startup error returns now wait for connection cleanup.
Normal successful connections retain their configured optimize-on-close behavior.
Cancellation/dropping of an in-flight connection future is not changed by this PR.

Verification

Local environment: macOS arm64, repository toolchain Rust 1.94.1, libsqlite3-sys 0.37.0, bundled SQLite 3.51.3.

  • Reproduced premature error return against unchanged upstream before fixing the driver.
  • The final synchronized regression rejects replacing awaited shutdown with plain drop(conn).
  • The callback-reference assertion independently failed before the collation validation-order fix.
  • cargo fmt --all -- --check passed.
  • cargo clippy -p sqlx-sqlite --all-targets --all-features -- -D warnings passed.
  • cargo test -p sqlx-sqlite --all-features passed: 14 unit tests, the new regression, and 4 doc tests.
  • The final cleanup regression passed 200 consecutive executions.
  • Existing Tokio SQLite integration suites passed: 174 tests across sqlite, sqlite-types, sqlite-describe, sqlite-error, sqlite-migrate, and sqlite-derives.
  • Existing sqlite and sqlite-error suites passed under smol and async-global-executor: 50 tests per runtime.
  • Each run of the existing sqlite suite retains its one pre-existing ignored test, issue_1467.
  • The same regression source also passed in an isolated unbundled consumer linked against Homebrew SQLite with async-global-executor enabled.
    Apple's built-in SQLite rejected auto-extension registration, so that library is not counted as passing this fixture.

Integration tests used disposable copies of the checked-in SQLite fixture, not retained application data.
CI and upstream review remain required before merging.

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.

1 participant