Skip to content

Finalize SQLite statement before releasing control arguments - #38

Merged
simolus3 merged 1 commit into
powersync-ja:mainfrom
GuionAI:fix/finalize-statement-before-control-argument
Sep 17, 2026
Merged

simolus3 merged 1 commit into
powersync-ja:mainfrom
GuionAI:fix/finalize-statement-before-control-argument

Conversation

@birdmanmandbir

@birdmanmandbir birdmanmandbir commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Construct the owned control argument before its SQLite statement, so every return and unwind path finalizes the statement before the argument can be released.
  • Keep the SQLite STATIC binding semantics from Scope SQLite control argument lifetime #37 and document the all-exit lifetime reasoning at the unsafe call site.
  • Keep coverage in the existing sync-client integration tests instead of duplicating the same control path locally.

Root cause

#37 explicitly finalized the statement on its successful path, but the statement was declared before the owned argument. Any later error path dropped the argument before ManagedStmt finalized SQLite, which violates SQLite STATIC binding requirements.

Validation

  • cargo fmt --all --check
  • cargo test --workspace --all-features

Follow-up to #37; no separate issue because this is a narrowly scoped correction to that merged PR.

@simolus3 simolus3 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this fix is better 👍

Comment thread powersync/src/sync/download/sync_iteration.rs Outdated
Comment thread powersync/src/sync/download/sync_iteration.rs Outdated
@orga-agent
orga-agent Bot force-pushed the fix/finalize-statement-before-control-argument branch from d4f1d6e to 3612a6c Compare September 17, 2026 03:37
@simolus3
simolus3 merged commit 67f7584 into powersync-ja:main Sep 17, 2026
2 checks passed
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