fix: scope database connections to their NitroSQLite owner - #421
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NitroSQLite keeps database connections in a process-wide registry after their JavaScript runtime is destroyed. A replacement runtime then fails to reopen the same database with an "already open" error, which prevents Onyx from loading the persisted session in the Expensify deploy blocker. This change makes each native NitroSQLite root own its default and independent connections and close them on destruction.
Queries, batches, file imports, and prepared statements resolve through their owning root. Pending work retains its original connection and fails after closure. A newer root can open the same database name, and delayed cleanup from an older root cannot close its handles. Weak references preserve the existing migration and deletion safeguards across overlapping roots without keeping old connections alive.
The grouped-batch tests added on main exposed an empty-array conversion error during integration. The native converter now skips an empty flat parameter array as well as an empty nested group, preserving the behavior those tests require. This correction is separate from the ownership backports for 9.8.3 and 10.0.1.
The connection regression tests cover owner destruction, retained handles, overlapping owners, WAL persistence, unfinished transaction rollback, and 100 reopen cycles. A separate native lifecycle check fails on unmodified main with the repeat-open error and passes with this fix, including stale async work and prepared statements. That check uses the real implementation and bundled SQLite with binding-only shims and a deferred scheduler.
Reproduction
For Expensify, repeat New Expensify to Classic to New Expensify transitions, with and without reset, and verify the session and offline data survive. The full Expensify Android staging flow and iOS runtime teardown remain unverified. Expensify needs a dependency update and native rebuild to consume this fix.