Skip to content

Commit fdfbd2c

Browse files
authored
fix: deadlock (#460)
after using it for a day, i saw two deadlocks. not exactly sure where they occur, but this is the only place i see potential issues. this must be coming from the dashmap replacement... Mutex requires us to be more careful.
1 parent 544b6b0 commit fdfbd2c

File tree

2 files changed

+36
-44
lines changed

2 files changed

+36
-44
lines changed

crates/pgt_workspace/src/workspace/server/connection_manager.rs

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -34,23 +34,19 @@ impl ConnectionManager {
3434
pub(crate) fn get_pool(&self, settings: &DatabaseSettings) -> Option<PgPool> {
3535
let key = ConnectionKey::from(settings);
3636

37-
// Cleanup idle connections first
38-
self.cleanup_idle_pools(&key);
39-
4037
if !settings.enable_connection {
4138
tracing::info!("Database connection disabled.");
4239
return None;
4340
}
4441

45-
// Try read lock first for cache hit
46-
if let Ok(pools) = self.pools.read() {
47-
if let Some(cached_pool) = pools.get(&key) {
48-
// Can't update last_accessed with read lock, but that's okay for occasional misses
49-
return Some(cached_pool.pool.clone());
42+
{
43+
if let Ok(pools) = self.pools.read() {
44+
if let Some(cached_pool) = pools.get(&key) {
45+
return Some(cached_pool.pool.clone());
46+
}
5047
}
5148
}
5249

53-
// Cache miss or need to update timestamp - use write lock
5450
let mut pools = self.pools.write().unwrap();
5551

5652
// Double-check after acquiring write lock
@@ -59,6 +55,21 @@ impl ConnectionManager {
5955
return Some(cached_pool.pool.clone());
6056
}
6157

58+
// Clean up idle connections before creating new ones to avoid unbounded growth
59+
let now = Instant::now();
60+
pools.retain(|k, cached_pool| {
61+
let idle_duration = now.duration_since(cached_pool.last_accessed);
62+
if idle_duration > cached_pool.idle_timeout && k != &key {
63+
tracing::debug!(
64+
"Removing idle database connection (idle for {:?})",
65+
idle_duration
66+
);
67+
false
68+
} else {
69+
true
70+
}
71+
});
72+
6273
// Create a new pool
6374
let config = PgConnectOptions::new()
6475
.host(&settings.host)
@@ -85,25 +96,4 @@ impl ConnectionManager {
8596

8697
Some(pool)
8798
}
88-
89-
/// Remove pools that haven't been accessed for longer than the idle timeout
90-
fn cleanup_idle_pools(&self, ignore_key: &ConnectionKey) {
91-
let now = Instant::now();
92-
93-
let mut pools = self.pools.write().unwrap();
94-
95-
// Use retain to keep only non-idle connections
96-
pools.retain(|key, cached_pool| {
97-
let idle_duration = now.duration_since(cached_pool.last_accessed);
98-
if idle_duration > cached_pool.idle_timeout && key != ignore_key {
99-
tracing::debug!(
100-
"Removing idle database connection (idle for {:?})",
101-
idle_duration
102-
);
103-
false
104-
} else {
105-
true
106-
}
107-
});
108-
}
10999
}

crates/pgt_workspace/src/workspace/server/tree_sitter.rs

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,27 +28,29 @@ impl TreeSitterStore {
2828
}
2929

3030
pub fn get_or_cache_tree(&self, statement: &StatementId) -> Arc<tree_sitter::Tree> {
31-
let mut cache = self.db.lock().expect("Failed to lock cache");
32-
33-
if let Some(existing) = cache.get(statement) {
34-
return existing.clone();
31+
// First check cache
32+
{
33+
let mut cache = self.db.lock().unwrap();
34+
if let Some(existing) = cache.get(statement) {
35+
return existing.clone();
36+
}
3537
}
3638

37-
// Cache miss - drop cache lock, parse, then re-acquire to insert
38-
drop(cache);
39-
40-
let mut parser = self.parser.lock().expect("Failed to lock parser");
39+
// Cache miss - parse outside of cache lock to avoid deadlock
40+
let mut parser = self.parser.lock().unwrap();
4141
let tree = Arc::new(parser.parse(statement.content(), None).unwrap());
4242
drop(parser);
4343

44-
let mut cache = self.db.lock().expect("Failed to lock cache");
45-
46-
// Double-check after re-acquiring lock
47-
if let Some(existing) = cache.get(statement) {
48-
return existing.clone();
44+
// Insert into cache
45+
{
46+
let mut cache = self.db.lock().unwrap();
47+
// Double-check in case another thread inserted while we were parsing
48+
if let Some(existing) = cache.get(statement) {
49+
return existing.clone();
50+
}
51+
cache.put(statement.clone(), tree.clone());
4952
}
5053

51-
cache.put(statement.clone(), tree.clone());
5254
tree
5355
}
5456
}

0 commit comments

Comments
 (0)