Skip to content

Commit c7bd063

Browse files
committed
fix(sqlite): resolve deadlock in collect_iterator
The collect_iterator method was causing a deadlock when filter_map_fn called storage.get() because std::sync::Mutex is not reentrant. The method held the mutex lock during iteration, and nested get() calls would try to acquire the same lock, causing tests to hang indefinitely. Fix by collecting all matching rows first while holding the lock, then releasing the lock before processing rows with filter_map_fn.
1 parent 88fc3c2 commit c7bd063

File tree

1 file changed

+34
-20
lines changed

1 file changed

+34
-20
lines changed

light-client-lib/src/storage/db/sqlite.rs

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -195,29 +195,43 @@ impl StorageBackend for Storage {
195195
skip: usize,
196196
limit: usize,
197197
) -> Vec<KVPair> {
198-
let conn = self.conn.lock().unwrap();
199-
200-
let (order_clause, comparison) = match direction {
201-
IteratorDirection::Forward => ("ASC", ">="),
202-
IteratorDirection::Reverse => ("DESC", "<="),
203-
};
204-
205-
let query = format!(
206-
"SELECT key, value FROM kv_store WHERE key {} ? ORDER BY key {}",
207-
comparison, order_clause
208-
);
198+
// First, collect all matching rows while holding the lock.
199+
// This is necessary because filter_map_fn may call storage.get() which needs the lock,
200+
// and std::sync::Mutex is not reentrant. By collecting rows first and releasing the lock,
201+
// we avoid deadlock when filter_map_fn tries to acquire the lock again.
202+
let rows: Vec<(Vec<u8>, Vec<u8>)> = {
203+
let conn = self.conn.lock().unwrap();
204+
205+
let (order_clause, comparison) = match direction {
206+
IteratorDirection::Forward => ("ASC", ">="),
207+
IteratorDirection::Reverse => ("DESC", "<="),
208+
};
209+
210+
let query = format!(
211+
"SELECT key, value FROM kv_store WHERE key {} ? ORDER BY key {}",
212+
comparison, order_clause
213+
);
209214

210-
let mut stmt = conn.prepare(&query).expect("prepare query");
215+
let mut stmt = conn.prepare(&query).expect("prepare query");
211216

212-
let rows = stmt
213-
.query_map(rusqlite::params![&from_key], |row| {
214-
Ok((row.get::<_, Vec<u8>>(0)?, row.get::<_, Vec<u8>>(1)?))
215-
})
216-
.expect("query rows");
217+
let query_rows = stmt
218+
.query_map(rusqlite::params![&from_key], |row| {
219+
Ok((row.get::<_, Vec<u8>>(0)?, row.get::<_, Vec<u8>>(1)?))
220+
})
221+
.expect("query rows");
222+
223+
// Collect rows that pass the take_while condition.
224+
// We need to eagerly collect because the lock must be released before filter_map_fn runs.
225+
query_rows
226+
.filter_map(|r| r.ok())
227+
.skip(skip)
228+
.take_while(|(key, _)| take_while_fn(key))
229+
.collect()
230+
};
231+
// Lock is released here
217232

218-
rows.filter_map(|r| r.ok())
219-
.skip(skip)
220-
.take_while(|(key, _)| take_while_fn(key))
233+
// Now process rows with filter_map_fn (which may call storage.get())
234+
rows.into_iter()
221235
.filter_map(|(key, value)| {
222236
filter_map_fn(&key, &value).map(|transformed_value| KVPair {
223237
key,

0 commit comments

Comments
 (0)