projects/guix·blob python-team · refs

320 lines · 11.6 KiB

1From: Danny Milosavljevic <[email protected]>
2Date: Fri, 6 Mar 2026 21:06:00 +0000
3Subject: [PATCH] sqlez: Fix named in-memory SQLite mode
4License: expat
5
6Zed's named "in-memory" SQLite databases were being opened with
7URI-looking paths such as `file:DB?mode=memory&cache=shared` but without
8`SQLITE_OPEN_URI`. SQLite therefore treated those strings as literal
9filenames, which leaked test databases to disk and defeated the intended
10shared-memory behavior.
11
12Fix that by opening named in-memory databases with `SQLITE_OPEN_URI` so
13`mode=memory&cache=shared` is interpreted correctly, and add coverage to
14ensure no literal backing files are created.
15
16That URI fix makes shared-cache schema locking real, which in turn
17exposes a second bug in `ThreadSafeConnection`: per-thread connection
18initialization can race schema setup on another connection to the same
19named in-memory database and fail while preparing `PRAGMA foreign_keys`.
20
21Fix that by retrying connection initialization queries when SQLite
22reports a transient schema/database lock, and add a regression test for
23that case.
24
25diff --git a/crates/sqlez/src/connection.rs b/crates/sqlez/src/connection.rs
26index 53f0d4e261..3dc530757a 100644
27--- a/crates/sqlez/src/connection.rs
28+++ b/crates/sqlez/src/connection.rs
29@@ -18,7 +18,7 @@ pub struct Connection {
30 unsafe impl Send for Connection {}
31
32 impl Connection {
33- pub(crate) fn open(uri: &str, persistent: bool) -> Result<Self> {
34+ fn open_with_flags(uri: &str, persistent: bool, flags: i32) -> Result<Self> {
35 let mut connection = Self {
36 sqlite3: ptr::null_mut(),
37 persistent,
38@@ -26,7 +26,6 @@ impl Connection {
39 _sqlite: PhantomData,
40 };
41
42- let flags = SQLITE_OPEN_CREATE | SQLITE_OPEN_NOMUTEX | SQLITE_OPEN_READWRITE;
43 unsafe {
44 sqlite3_open_v2(
45 CString::new(uri)?.as_ptr(),
46@@ -44,6 +43,14 @@ impl Connection {
47 Ok(connection)
48 }
49
50+ pub(crate) fn open(uri: &str, persistent: bool) -> Result<Self> {
51+ Self::open_with_flags(
52+ uri,
53+ persistent,
54+ SQLITE_OPEN_CREATE | SQLITE_OPEN_NOMUTEX | SQLITE_OPEN_READWRITE,
55+ )
56+ }
57+
58 /// Attempts to open the database at uri. If it fails, a shared memory db will be opened
59 /// instead.
60 pub fn open_file(uri: &str) -> Self {
61@@ -51,13 +58,20 @@ impl Connection {
62 }
63
64 pub fn open_memory(uri: Option<&str>) -> Self {
65- let in_memory_path = if let Some(uri) = uri {
66- format!("file:{}?mode=memory&cache=shared", uri)
67+ if let Some(uri) = uri {
68+ let in_memory_path = format!("file:{}?mode=memory&cache=shared", uri);
69+ return Self::open_with_flags(
70+ &in_memory_path,
71+ false,
72+ SQLITE_OPEN_CREATE
73+ | SQLITE_OPEN_NOMUTEX
74+ | SQLITE_OPEN_READWRITE
75+ | SQLITE_OPEN_URI,
76+ )
77+ .expect("Could not create fallback in memory db");
78 } else {
79- ":memory:".to_string()
80- };
81-
82- Self::open(&in_memory_path, false).expect("Could not create fallback in memory db")
83+ Self::open(":memory:", false).expect("Could not create fallback in memory db")
84+ }
85 }
86
87 pub fn persistent(&self) -> bool {
88@@ -265,9 +279,50 @@ impl Drop for Connection {
89 mod test {
90 use anyhow::Result;
91 use indoc::indoc;
92+ use std::{
93+ fs,
94+ sync::atomic::{AtomicUsize, Ordering},
95+ };
96
97 use crate::connection::Connection;
98
99+ static NEXT_NAMED_MEMORY_DB_ID: AtomicUsize = AtomicUsize::new(0);
100+
101+ fn unique_named_memory_db(prefix: &str) -> String {
102+ format!(
103+ "{prefix}_{}_{}",
104+ std::process::id(),
105+ NEXT_NAMED_MEMORY_DB_ID.fetch_add(1, Ordering::Relaxed)
106+ )
107+ }
108+
109+ fn literal_named_memory_paths(name: &str) -> [String; 3] {
110+ let main = format!("file:{name}?mode=memory&cache=shared");
111+ [main.clone(), format!("{main}-wal"), format!("{main}-shm")]
112+ }
113+
114+ struct NamedMemoryPathGuard {
115+ paths: [String; 3],
116+ }
117+
118+ impl NamedMemoryPathGuard {
119+ fn new(name: &str) -> Self {
120+ let paths = literal_named_memory_paths(name);
121+ for path in &paths {
122+ let _ = fs::remove_file(path);
123+ }
124+ Self { paths }
125+ }
126+ }
127+
128+ impl Drop for NamedMemoryPathGuard {
129+ fn drop(&mut self) {
130+ for path in &self.paths {
131+ let _ = fs::remove_file(path);
132+ }
133+ }
134+ }
135+
136 #[test]
137 fn string_round_trips() -> Result<()> {
138 let connection = Connection::open_memory(Some("string_round_trips"));
139@@ -382,6 +437,41 @@ mod test {
140 assert_eq!(read_blobs, vec![blob]);
141 }
142
143+ #[test]
144+ fn named_memory_connections_do_not_create_literal_backing_files() {
145+ let name = unique_named_memory_db("named_memory_connections_do_not_create_backing_files");
146+ let guard = NamedMemoryPathGuard::new(&name);
147+
148+ let connection1 = Connection::open_memory(Some(&name));
149+ connection1
150+ .exec(indoc! {"
151+ CREATE TABLE shared (
152+ value INTEGER
153+ )"})
154+ .unwrap()()
155+ .unwrap();
156+ connection1
157+ .exec("INSERT INTO shared (value) VALUES (7)")
158+ .unwrap()()
159+ .unwrap();
160+
161+ let connection2 = Connection::open_memory(Some(&name));
162+ assert_eq!(
163+ connection2
164+ .select_row::<i64>("SELECT value FROM shared")
165+ .unwrap()()
166+ .unwrap(),
167+ Some(7)
168+ );
169+
170+ for path in &guard.paths {
171+ assert!(
172+ fs::metadata(path).is_err(),
173+ "named in-memory database unexpectedly created backing file {path}"
174+ );
175+ }
176+ }
177+
178 #[test]
179 fn multi_step_statement_works() {
180 let connection = Connection::open_memory(Some("multi_step_statement_works"));
181diff --git a/crates/sqlez/src/thread_safe_connection.rs b/crates/sqlez/src/thread_safe_connection.rs
182index 966f14a9c2..9d868fcba4 100644
183--- a/crates/sqlez/src/thread_safe_connection.rs
184+++ b/crates/sqlez/src/thread_safe_connection.rs
185@@ -7,12 +7,15 @@ use std::{
186 ops::Deref,
187 sync::{Arc, LazyLock},
188 thread,
189+ time::Duration,
190 };
191 use thread_local::ThreadLocal;
192
193 use crate::{connection::Connection, domain::Migrator, util::UnboundedSyncSender};
194
195 const MIGRATION_RETRIES: usize = 10;
196+const CONNECTION_INITIALIZE_RETRIES: usize = 50;
197+const CONNECTION_INITIALIZE_RETRY_DELAY: Duration = Duration::from_millis(1);
198
199 type QueuedWrite = Box<dyn 'static + Send + FnOnce()>;
200 type WriteQueue = Box<dyn 'static + Send + Sync + Fn(QueuedWrite)>;
201@@ -197,21 +200,50 @@ impl ThreadSafeConnection {
202 Self::open_shared_memory(uri)
203 };
204
205+ if let Some(initialize_query) = connection_initialize_query {
206+ let mut last_error = None;
207+ let initialized = (0..CONNECTION_INITIALIZE_RETRIES).any(|attempt| {
208+ match connection.exec(initialize_query).and_then(|mut statement| statement()) {
209+ Ok(()) => true,
210+ Err(err)
211+ if is_schema_lock_error(&err)
212+ && attempt + 1 < CONNECTION_INITIALIZE_RETRIES =>
213+ {
214+ last_error = Some(err);
215+ thread::sleep(CONNECTION_INITIALIZE_RETRY_DELAY);
216+ false
217+ }
218+ Err(err) => {
219+ panic!(
220+ "Initialize query failed to execute: {}\n\nCaused by:\n{err:#}",
221+ initialize_query
222+ )
223+ }
224+ }
225+ });
226+
227+ if !initialized {
228+ let err = last_error.expect("connection initialization retries should record the last error");
229+ panic!(
230+ "Initialize query failed to execute after retries: {}\n\nCaused by:\n{err:#}",
231+ initialize_query
232+ );
233+ }
234+ }
235+
236 // Disallow writes on the connection. The only writes allowed for thread safe connections
237 // are from the background thread that can serialize them.
238 *connection.write.get_mut() = false;
239
240- if let Some(initialize_query) = connection_initialize_query {
241- connection.exec(initialize_query).unwrap_or_else(|_| {
242- panic!("Initialize query failed to execute: {}", initialize_query)
243- })()
244- .unwrap()
245- }
246-
247 connection
248 }
249 }
250
251+fn is_schema_lock_error(err: &anyhow::Error) -> bool {
252+ let message = format!("{err:#}");
253+ message.contains("database schema is locked") || message.contains("database is locked")
254+}
255+
256 impl ThreadSafeConnection {
257 /// Special constructor for ThreadSafeConnection which disallows db initialization and migrations.
258 /// This allows construction to be infallible and not write to the db.
259@@ -282,7 +314,7 @@ mod test {
260 use indoc::indoc;
261 use std::ops::Deref;
262
263- use std::thread;
264+ use std::{thread, time::Duration};
265
266 use crate::{domain::Domain, thread_safe_connection::ThreadSafeConnection};
267
268@@ -318,38 +350,21 @@ mod test {
269 }
270
271 #[test]
272- #[should_panic]
273- fn wild_zed_lost_failure() {
274- enum TestWorkspace {}
275- impl Domain for TestWorkspace {
276- const NAME: &str = "workspace";
277-
278- const MIGRATIONS: &[&str] = &["
279- CREATE TABLE workspaces(
280- workspace_id INTEGER PRIMARY KEY,
281- dock_visible INTEGER, -- Boolean
282- dock_anchor TEXT, -- Enum: 'Bottom' / 'Right' / 'Expanded'
283- dock_pane INTEGER, -- NULL indicates that we don't have a dock pane yet
284- timestamp TEXT DEFAULT CURRENT_TIMESTAMP NOT NULL,
285- FOREIGN KEY(dock_pane) REFERENCES panes(pane_id),
286- FOREIGN KEY(active_pane) REFERENCES panes(pane_id)
287- ) STRICT;
288-
289- CREATE TABLE panes(
290- pane_id INTEGER PRIMARY KEY,
291- workspace_id INTEGER NOT NULL,
292- active INTEGER NOT NULL, -- Boolean
293- FOREIGN KEY(workspace_id) REFERENCES workspaces(workspace_id)
294- ON DELETE CASCADE
295- ON UPDATE CASCADE
296- ) STRICT;
297- "];
298- }
299+ fn connection_initialize_query_retries_transient_schema_lock() {
300+ let name = "connection_initialize_query_retries_transient_schema_lock";
301+ let locking_connection = crate::connection::Connection::open_memory(Some(name));
302+ locking_connection.exec("BEGIN IMMEDIATE").unwrap()().unwrap();
303+ locking_connection
304+ .exec("CREATE TABLE test(col TEXT)")
305+ .unwrap()()
306+ .unwrap();
307
308- let builder =
309- ThreadSafeConnection::builder::<TestWorkspace>("wild_zed_lost_failure", false)
310- .with_connection_initialize_query("PRAGMA FOREIGN_KEYS=true");
311+ let releaser = thread::spawn(move || {
312+ thread::sleep(Duration::from_millis(10));
313+ locking_connection.exec("ROLLBACK").unwrap()().unwrap();
314+ });
315
316- smol::block_on(builder.build()).unwrap();
317+ ThreadSafeConnection::create_connection(false, name, Some("PRAGMA FOREIGN_KEYS=true"));
318+ releaser.join().unwrap();
319 }
320 }