Skip to content

fix: deadlock #460

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 20 additions & 30 deletions crates/pgt_workspace/src/workspace/server/connection_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,19 @@ impl ConnectionManager {
pub(crate) fn get_pool(&self, settings: &DatabaseSettings) -> Option<PgPool> {
let key = ConnectionKey::from(settings);

// Cleanup idle connections first
self.cleanup_idle_pools(&key);

if !settings.enable_connection {
tracing::info!("Database connection disabled.");
return None;
}

// Try read lock first for cache hit
if let Ok(pools) = self.pools.read() {
if let Some(cached_pool) = pools.get(&key) {
// Can't update last_accessed with read lock, but that's okay for occasional misses
return Some(cached_pool.pool.clone());
{
if let Ok(pools) = self.pools.read() {
if let Some(cached_pool) = pools.get(&key) {
return Some(cached_pool.pool.clone());
}
}
}

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

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

// Clean up idle connections before creating new ones to avoid unbounded growth
let now = Instant::now();
pools.retain(|k, cached_pool| {
let idle_duration = now.duration_since(cached_pool.last_accessed);
if idle_duration > cached_pool.idle_timeout && k != &key {
tracing::debug!(
"Removing idle database connection (idle for {:?})",
idle_duration
);
false
} else {
true
}
});

// Create a new pool
let config = PgConnectOptions::new()
.host(&settings.host)
Expand All @@ -85,25 +96,4 @@ impl ConnectionManager {

Some(pool)
}

/// Remove pools that haven't been accessed for longer than the idle timeout
fn cleanup_idle_pools(&self, ignore_key: &ConnectionKey) {
let now = Instant::now();

let mut pools = self.pools.write().unwrap();

// Use retain to keep only non-idle connections
pools.retain(|key, cached_pool| {
let idle_duration = now.duration_since(cached_pool.last_accessed);
if idle_duration > cached_pool.idle_timeout && key != ignore_key {
tracing::debug!(
"Removing idle database connection (idle for {:?})",
idle_duration
);
false
} else {
true
}
});
}
}
30 changes: 16 additions & 14 deletions crates/pgt_workspace/src/workspace/server/tree_sitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,29 @@ impl TreeSitterStore {
}

pub fn get_or_cache_tree(&self, statement: &StatementId) -> Arc<tree_sitter::Tree> {
let mut cache = self.db.lock().expect("Failed to lock cache");

if let Some(existing) = cache.get(statement) {
return existing.clone();
// First check cache
{
let mut cache = self.db.lock().unwrap();
if let Some(existing) = cache.get(statement) {
return existing.clone();
}
}

// Cache miss - drop cache lock, parse, then re-acquire to insert
drop(cache);

let mut parser = self.parser.lock().expect("Failed to lock parser");
// Cache miss - parse outside of cache lock to avoid deadlock
let mut parser = self.parser.lock().unwrap();
let tree = Arc::new(parser.parse(statement.content(), None).unwrap());
drop(parser);

let mut cache = self.db.lock().expect("Failed to lock cache");

// Double-check after re-acquiring lock
if let Some(existing) = cache.get(statement) {
return existing.clone();
// Insert into cache
{
let mut cache = self.db.lock().unwrap();
// Double-check in case another thread inserted while we were parsing
if let Some(existing) = cache.get(statement) {
return existing.clone();
}
cache.put(statement.clone(), tree.clone());
}

cache.put(statement.clone(), tree.clone());
tree
}
}