Skip to content

libsql: Use RwLock instead of RefCell for local connection#2118

Merged
penberg merged 2 commits intotursodatabase:mainfrom
Resonious:main
Jul 29, 2025
Merged

libsql: Use RwLock instead of RefCell for local connection#2118
penberg merged 2 commits intotursodatabase:mainfrom
Resonious:main

Conversation

@Resonious
Copy link
Copy Markdown
Contributor

@Resonious Resonious commented Jul 7, 2025

The local::Connection struct has the following safety comments:

// SAFETY: This is safe because we compile sqlite3 w/ SQLITE_THREADSAFE=1
unsafe impl Send for Connection {}
// SAFETY: This is safe because we compile sqlite3 w/ SQLITE_THREADSAFE=1
unsafe impl Sync for Connection {}

This is correct with respect to the *mut ffi::sqlite3 field, but NOT with respect to the RefCell<Option<AuthHook>> field. RefCell is not thread safe. Heavy load in a real application using local databases occasionally sees panics due to concurrent access to this RefCell.

I don't have a simple repro right now, but this change eliminated the panics for me. Let me know if you'd like some extra work done like a repro or tests (not sure I'll have time but I will try..)

@Resonious Resonious changed the title Use RwLock instead of RefCell for local connection [libsql crate] RwLock instead of RefCell for local connection Jul 7, 2025
@Resonious Resonious changed the title [libsql crate] RwLock instead of RefCell for local connection [libsql crate] Use RwLock instead of RefCell for local connection Jul 7, 2025
The `local::Connection` struct has the following safety comments:

```rust
// SAFETY: This is safe because we compile sqlite3 w/ SQLITE_THREADSAFE=1
unsafe impl Send for Connection {}
// SAFETY: This is safe because we compile sqlite3 w/ SQLITE_THREADSAFE=1
unsafe impl Sync for Connection {}
```

This is correct with respect to the `*mut ffi::sqlite3` field, but NOT
with respect to the `RefCell<Option<AuthHook>>` field. Heavy load in
a real application using local databases occasionally sees panics due
to concurrent access to this RefCell.
@penberg penberg changed the title [libsql crate] Use RwLock instead of RefCell for local connection libsql: Use RwLock instead of RefCell for local connection Jul 29, 2025
@penberg penberg enabled auto-merge July 29, 2025 10:10
@penberg penberg added this pull request to the merge queue Jul 29, 2025
Merged via the queue into tursodatabase:main with commit a9c66e0 Jul 29, 2025
19 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