apache / apache/doris-mcp-server
Global connection pool recovery deadlocks by reacquiring the same asyncio.Lock
- Dominant language
- Python
- Stars
- 345
- Forks
- 101
- PR merge metrics
- No merged PRs in 30d
Description
## Bug description
Global connection-pool recovery can deadlock indefinitely when the pool is unavailable, closed, or acquisition times out.
This was observed after a transient Doris connectivity failure during HTTP server startup. The server continued running without a global pool, but every later query—including `SELECT 1`—waited until the tool-level timeout. A fresh `aiomysql` connection from the same container could execute `SELECT 1` successfully, and restarting the container immediately restored service by creating the pool during startup.
The locking problem is still present on current `master` (`5daf1deb26bc0db02c19bf5ca1d070acea4cfab9`).
## Root cause
`DorisConnectionManager` aliases both lock names to the same non-reentrant `asyncio.Lock`:
```python
self._recovery_lock = asyncio.Lock()
self.pool_recovery_lock = self._recovery_lock
```
`_recover_pool_with_lock()` acquires `_recovery_lock` and then calls `_recover_pool()`:
```python
async def _recover_pool_with_lock(self) -> None:
async with self._recovery_lock:
if not self.pool_recovering:
await self._recover_pool()
```
However, `_recover_pool()` tries to acquire `pool_recovery_lock`, which is the same lock:
```python
async def _recover_pool(self) -> None:
async with self.pool_recovery_lock:
...
```
Because `asyncio.Lock` is not reentrant, `_recover_pool()` waits forever for a lock held by its caller.
The affected paths include:
- no global pool: `get_connection()` -> `_recover_pool_with_lock()`
- closed global pool: `get_connection()` -> `_recover_pool_with_lock()`
- pool acquisition timeout: `get_connection()` -> `_recover_pool_with_lock()`
## Observed behavior
The startup connectivity check encountered a transient failure:
```text
Database connectivity test failed: (2013, 'Lost connection to MySQL server during query')
HTTP mode running without global database pool, will use token-bound configurations
```
Later queries logged:
```text
Connection pool is not available, attempting recovery...
Query execution failed: Query timeout after 10 seconds
```
Notably, the expected recovery log below never appeared, because execution deadlocked before entering the body of `_recover_pool()`:
```text
Attempting pool recovery (attempt 1/3)
```
## Minimal deterministic reproduction
This reproduces without a live Doris instance because the deadlock occurs before any database operation:
```python
import asyncio
from doris_mcp_server.utils.db import DorisConnectionManager
async def main():
manager = object.__new__(DorisConnectionManager)
manager._recovery_lock = asyncio.Lock()
manager.pool_recovery_lock = manager._recovery_lock
manager.pool_recovering = False
# Raises TimeoutError; expected to enter recovery and complete/fail normally.
await asyncio.wait_for(manager._recover_pool_with_lock(), timeout=0.1)
asyncio.run(main())
```
## Expected behavior
Recovery should have a single lock owner and should either recreate the pool or return a bounded error. It must not deadlock.
## Suggested fix
Remove the nested acquisition. For example, make `_recover_pool()` the only method responsible for acquiring the recovery lock, and have callers invoke it directly (or make `_recover_pool_with_lock()` delegate without acquiring the same lock).
Please also add regression tests covering:
1. recovery when `self.pool is None`;
2. recovery when the pool is closed;
3. recovery after acquisition timeout;
4. concurrent recovery requests, verifying bounded completion and only one active recovery.
## Environment
- Transport: Streamable HTTP
- Deployment: Docker Compose, one worker
- Observed release: `0.6.1` / commit `81305ffbf97482e29abf70dc0ba9be928ed39df5`
- Confirmed by source inspection on current `master`: `5daf1deb26bc0db02c19bf5ca1d070acea4cfab9`
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in doris_mcp_server/utils/db.py with DorisConnectionManager._recover_pool_with_lock(), _recover_pool(), and get_connection(), then run the deterministic asyncio reproduction from the issue. Verify that pool recovery completes or fails within a bounded time, concurrent requests allow only one active recovery, and regression tests cover missing, closed, and timed-out pools.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100