fix(app): recursive cursor errors #7727
Merged
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.
Summary
In #7724, we enabled WAL mode (which is the suggested way to use SQLite since 2010) and removed the SQLite global mutex. This inadvertently exposed some tech debt/bad practices with SQLite.
Throughout the app, each service would create a single SQLite "cursor" at init and re-use it through the app's lifetime.
The global mutex accidentally prevented an issue where a cursor was used multiple times at once. With it gone, it's possible for a DB operation to fail with an error like this:
Cursors are meant to be lightweight and exist only for short periods of time. This issue is one reason why cursors are not meant to be long-lived entities, shared across a broad scope.
This PR pays off the tech debt by creating a new cursor in all service methods that need a cursor.
It also removes more extraneous class attribute type annotations, which could obscure a runtime issue by telling the type checker that a class attribute exists even if it was never set.
This fixes errors like this:
Related Issues / Discussions
Initially reported by @mickr777 on discord: https://discord.com/channels/1020123559063990373/1346012301056151663
QA Instructions
@mickr777 already confirmed this fixes their issue. I did some smoke tests and do not get any errors, but also I cannot repro it despite following the same steps as outlined in the discord thread.
Merge Plan
n/a
Checklist
What's New
copy (if doing a release after this PR)