[LucaCappelletti94]

Concurrent installers must agree on one memvfs registration and backing store after Spxg#204 makes file state shareable. With threadsafe, one critical section covers lookup and registry publication, while the public entry points remain unsafe.

OS callbacks may reenter install() from their destructors. Allocation failure must release registration synchronization before destroying them, without allocating candidate state for an installation that can reuse the existing VFS.

This work is primarily towards removing the need to mark the involved methods as unsafe, which when multi-threading is enabled they should not be as far as I understand. The safe switch will be stacked in the next PR on top.

Concurrent memvfs installations could create separate registrations and backing stores. With threadsafe, the fix serializes registration so installers reuse one VFS.

If installation fails, destroying an OS callback while holding the registration mutex can deadlock when its destructor reenters install(). The fix releases the mutex before destroying rejected callbacks.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LucaCappelletti94/coderabbit/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 8f63fe66-1955-414e-871c-e26f1f8c0ccb

📥 Commits

Reviewing files that changed from the base of the PR and between 2172557 and abb50c3.

📒 Files selected for processing: 5

• CHANGELOG.md
• crates/rsqlite-vfs/src/ffi.rs
• crates/rsqlite-vfs/src/memvfs.rs
• crates/rsqlite-vfs/src/sync.rs
• crates/rsqlite-vfs/tests/memvfs_registration.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The thread-safe memvfs path now initializes SQLite and serializes registration with a guard. Installation prepares and publishes ownership state with acquire/release ordering. Tests cover mutex-allocation failure with callback reentry and concurrent database import and export.

Changes: Thread-safe memvfs registration

Layer / File(s)	Summary
Initialize SQLite and serialize registration crates/rsqlite-vfs/src/ffi.rs, crates/rsqlite-vfs/src/sync.rs, crates/rsqlite-vfs/src/memvfs.rs	Adds the sqlite3_initialize declaration and a RegistrationGuard that initializes SQLite and enters the static VFS2 mutex. Installation enters the guard when threadsafe is enabled.
Publish and clear VFS ownership crates/rsqlite-vfs/src/memvfs.rs	Installation prepares the file map before publishing the VFS pointer. Registration failure clears ownership and releases allocations. Ownership checks and uninstallation use acquire/release ordering.
Exercise failure and concurrent registration crates/rsqlite-vfs/tests/memvfs_registration.rs, CHANGELOG.md	Integration tests cover mutex-allocation failure with callback reentry and concurrent database imports and exports. The changelog records the registration behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Installer
  participant RegistrationGuard
  participant SQLite
  participant MemVfs
  Installer->>RegistrationGuard: enter()
  RegistrationGuard->>SQLite: sqlite3_initialize()
  RegistrationGuard->>SQLite: allocate and enter static VFS2 mutex
  Installer->>MemVfs: install after guard acquisition
  MemVfs->>SQLite: register VFS
  SQLite-->>MemVfs: registration result

Loading

Suggested reviewers: spxg

Merge Risk: ⚪ Minimal · up to abb50

No identified issue blocks merging the registration change after normal checks.

Pre-merge checks: 11 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 4 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 11 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title uses an imperative verb, describes the main change, contains no file path or conventional-commit prefix, has no trailing period, and is under 70 characters.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
No Placeholder Implementations		No added line contains todo!, unimplemented!, the specified panic!, NotImplementedError, a TypeScript not-implemented throw, or a new TODO, FIXME, HACK, or XXX comment. The added `expe…
No Blanket Diagnostic Suppression		No added diagnostic suppression matches the check. The Rust changes add FFI, synchronization, ownership, tests, and comments only; they add no #[allow(...)], #[deny(warnings)], `#![allow(warnings)…
Behavior Change Carries A Test		The pull request changes runtime behavior in crates/rsqlite-vfs/src/ffi.rs, memvfs.rs, and sync.rs. It also adds crates/rsqlite-vfs/tests/memvfs_registration.rs with a #[test] that asserts a…
Git Dependency Pin Stays Out Of Commits		Cargo.lock is unchanged in the reviewed pull-request range. The check therefore passes regardless of manifest dependency declarations.
Crate Readme Is The Crate Documentation		The repository contains src/lib.rs, but the reviewed pull-request range changes neither README.md nor any src/lib.rs. The check does not apply.
Pre-Alpha Has No Deployments		The affected package rsqlite-vfs has version 0.2.0, not 0.0.0. The added lines contain no migration, deployment, rollout, compatibility-shim, or legacy-client wording.
Prose Punctuation		No added prose violates the punctuation rule. Added Markdown, comments, doc comments, and assertion messages contain no semicolons, em dashes, en dashes, punctuation dashes, curly quotes, or ellipsis …

Full details: No Placeholder Implementations

Explanation

No added line contains todo!, unimplemented!, the specified panic!, NotImplementedError, a TypeScript not-implemented throw, or a new TODO, FIXME, HACK, or XXX comment. The added expect and test assertions are not placeholder implementations.

Full details: No Blanket Diagnostic Suppression

Explanation

No added diagnostic suppression matches the check. The Rust changes add FFI, synchronization, ownership, tests, and comments only; they add no #[allow(...)], #[deny(warnings)], #![allow(warnings)], or broad lint suppression. The new test’s #![cfg(feature = "threadsafe")] is feature gating, not diagnostic suppression.

Full details: Behavior Change Carries A Test

Explanation

The pull request changes runtime behavior in crates/rsqlite-vfs/src/ffi.rs, memvfs.rs, and sync.rs. It also adds crates/rsqlite-vfs/tests/memvfs_registration.rs with a #[test] that asserts allocation-failure handling, reentrant installation, concurrent installers, shared database state, and uninstall success.

Full details: Prose Punctuation

Explanation

No added prose violates the punctuation rule. Added Markdown, comments, doc comments, and assertion messages contain no semicolons, em dashes, en dashes, punctuation dashes, curly quotes, or ellipsis glyphs. Hyphens occur only in compound terms such as extension-VFS, app-data, and allocation-failure.

Full details: Docstring Coverage

Explanation

Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 4 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}