fix: Make all Docker subprocess calls non-blocking
Previously only docker_health was fixed to use run_in_executor(), but all other Docker operations (docker_status, docker_start, docker_stop, docker_logs, docker_build, docker_cleanup) still used synchronous subprocess.run() which blocked the async event loop. This caused docker_auto_start(wait=True) to freeze the entire MCP server. Now _run_docker_cmd is async and runs subprocess calls in thread executor. All callers updated to use await.
This commit is contained in:
parent
f1986db6cc
commit
6662c8411a
@ -63,7 +63,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
|||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
- **instances_use Hanging:** Eliminated 4+ hour hangs by removing blocking HTTP call. Now uses lazy registration — just creates a stub entry, validates on first real tool call.
|
- **instances_use Hanging:** Eliminated 4+ hour hangs by removing blocking HTTP call. Now uses lazy registration — just creates a stub entry, validates on first real tool call.
|
||||||
- **Event Loop Blocking:** `docker_health` now runs HTTP checks in thread executor via `run_in_executor()`, preventing MCP server freeze during health polling.
|
- **All Docker Operations Non-Blocking:** ALL Docker subprocess calls (`docker ps`, `docker run`, `docker stop`, etc.) now run in thread executor via `run_in_executor()`. Previously only `docker_health` was fixed, but `docker_status`, `docker_start`, `docker_stop`, `docker_logs`, `docker_build`, and `docker_cleanup` still blocked the event loop. This caused `docker_auto_start(wait=True)` to freeze the MCP server.
|
||||||
- **Session Isolation:** `docker_stop` now validates container belongs to current session before stopping. `docker_cleanup` defaults to `session_only=True` to prevent cross-session interference.
|
- **Session Isolation:** `docker_stop` now validates container belongs to current session before stopping. `docker_cleanup` defaults to `session_only=True` to prevent cross-session interference.
|
||||||
- **Background Discovery Thread:** Fixed timeout from 30s to 0.5s for port scanning, reducing discovery cycle from 300s+ to ~15s.
|
- **Background Discovery Thread:** Fixed timeout from 30s to 0.5s for port scanning, reducing discovery cycle from 300s+ to ~15s.
|
||||||
- **Typedef/Variable Type Resolution:** Fixed `handle_typedef_create` and `handle_variable_rename` to use shared `resolve_data_type()` for builtin types (int, char, etc.).
|
- **Typedef/Variable Type Resolution:** Fixed `handle_typedef_create` and `handle_variable_rename` to use shared `resolve_data_type()` for builtin types (int, char, etc.).
|
||||||
|
|||||||
@ -240,10 +240,10 @@ class DockerMixin(MCPMixin):
|
|||||||
"""Check if Docker is available on the system."""
|
"""Check if Docker is available on the system."""
|
||||||
return shutil.which("docker") is not None
|
return shutil.which("docker") is not None
|
||||||
|
|
||||||
def _run_docker_cmd(
|
def _run_docker_cmd_sync(
|
||||||
self, args: List[str], check: bool = True, capture: bool = True
|
self, args: List[str], check: bool = True, capture: bool = True
|
||||||
) -> subprocess.CompletedProcess:
|
) -> subprocess.CompletedProcess:
|
||||||
"""Run a docker command.
|
"""Run a docker command synchronously (internal use only).
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
args: Command arguments (after 'docker')
|
args: Command arguments (after 'docker')
|
||||||
@ -261,6 +261,26 @@ class DockerMixin(MCPMixin):
|
|||||||
text=True,
|
text=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
async def _run_docker_cmd(
|
||||||
|
self, args: List[str], check: bool = True, capture: bool = True
|
||||||
|
) -> subprocess.CompletedProcess:
|
||||||
|
"""Run a docker command without blocking the event loop.
|
||||||
|
|
||||||
|
Uses run_in_executor to run subprocess in thread pool.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
args: Command arguments (after 'docker')
|
||||||
|
check: Raise exception on non-zero exit
|
||||||
|
capture: Capture stdout/stderr
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
CompletedProcess result
|
||||||
|
"""
|
||||||
|
loop = asyncio.get_event_loop()
|
||||||
|
return await loop.run_in_executor(
|
||||||
|
None, self._run_docker_cmd_sync, args, check, capture
|
||||||
|
)
|
||||||
|
|
||||||
def _run_compose_cmd(
|
def _run_compose_cmd(
|
||||||
self,
|
self,
|
||||||
args: List[str],
|
args: List[str],
|
||||||
@ -336,7 +356,7 @@ class DockerMixin(MCPMixin):
|
|||||||
f"{self.LABEL_PREFIX}.pid": str(os.getpid()),
|
f"{self.LABEL_PREFIX}.pid": str(os.getpid()),
|
||||||
}
|
}
|
||||||
|
|
||||||
def _find_containers_by_label(
|
async def _find_containers_by_label(
|
||||||
self,
|
self,
|
||||||
label_filter: Optional[str] = None,
|
label_filter: Optional[str] = None,
|
||||||
session_only: bool = False,
|
session_only: bool = False,
|
||||||
@ -359,7 +379,7 @@ class DockerMixin(MCPMixin):
|
|||||||
if label_filter:
|
if label_filter:
|
||||||
filter_args.extend(["--filter", f"label={self.LABEL_PREFIX}.{label_filter}"])
|
filter_args.extend(["--filter", f"label={self.LABEL_PREFIX}.{label_filter}"])
|
||||||
|
|
||||||
ps_result = self._run_docker_cmd(
|
ps_result = await self._run_docker_cmd(
|
||||||
[
|
[
|
||||||
"ps", "-a",
|
"ps", "-a",
|
||||||
*filter_args,
|
*filter_args,
|
||||||
@ -425,23 +445,23 @@ class DockerMixin(MCPMixin):
|
|||||||
|
|
||||||
# Check if docker daemon is running
|
# Check if docker daemon is running
|
||||||
try:
|
try:
|
||||||
self._run_docker_cmd(["info"], check=True)
|
await self._run_docker_cmd(["info"], check=True)
|
||||||
result["docker_running"] = True
|
result["docker_running"] = True
|
||||||
except (subprocess.CalledProcessError, FileNotFoundError):
|
except (subprocess.CalledProcessError, FileNotFoundError):
|
||||||
return result
|
return result
|
||||||
|
|
||||||
# Check for docker compose
|
# Check for docker compose
|
||||||
try:
|
try:
|
||||||
self._run_docker_cmd(["compose", "version"], check=True)
|
await self._run_docker_cmd(["compose", "version"], check=True)
|
||||||
result["compose_available"] = True
|
result["compose_available"] = True
|
||||||
except subprocess.CalledProcessError:
|
except subprocess.CalledProcessError:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
# List all GhydraMCP containers (from any session)
|
# List all GhydraMCP containers (from any session)
|
||||||
result["containers"] = self._find_containers_by_label()
|
result["containers"] = await self._find_containers_by_label()
|
||||||
|
|
||||||
# List containers from this session only
|
# List containers from this session only
|
||||||
result["session_containers"] = self._find_containers_by_label(session_only=True)
|
result["session_containers"] = await self._find_containers_by_label(session_only=True)
|
||||||
|
|
||||||
# Get port pool status
|
# Get port pool status
|
||||||
if self._port_pool:
|
if self._port_pool:
|
||||||
@ -449,7 +469,7 @@ class DockerMixin(MCPMixin):
|
|||||||
|
|
||||||
# Also check by name pattern for containers without labels
|
# Also check by name pattern for containers without labels
|
||||||
try:
|
try:
|
||||||
ps_result = self._run_docker_cmd(
|
ps_result = await self._run_docker_cmd(
|
||||||
[
|
[
|
||||||
"ps",
|
"ps",
|
||||||
"-a",
|
"-a",
|
||||||
@ -478,7 +498,7 @@ class DockerMixin(MCPMixin):
|
|||||||
|
|
||||||
# List GhydraMCP images
|
# List GhydraMCP images
|
||||||
try:
|
try:
|
||||||
images_result = self._run_docker_cmd(
|
images_result = await self._run_docker_cmd(
|
||||||
[
|
[
|
||||||
"images",
|
"images",
|
||||||
"--filter",
|
"--filter",
|
||||||
@ -557,7 +577,7 @@ class DockerMixin(MCPMixin):
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
# Check if container with this name already exists
|
# Check if container with this name already exists
|
||||||
check_result = self._run_docker_cmd(
|
check_result = await self._run_docker_cmd(
|
||||||
["ps", "-a", "-q", "-f", f"name=^{name}$"], check=False
|
["ps", "-a", "-q", "-f", f"name=^{name}$"], check=False
|
||||||
)
|
)
|
||||||
if check_result.stdout.strip():
|
if check_result.stdout.strip():
|
||||||
@ -567,7 +587,7 @@ class DockerMixin(MCPMixin):
|
|||||||
}
|
}
|
||||||
|
|
||||||
# Check if port is already in use by a non-pool container
|
# Check if port is already in use by a non-pool container
|
||||||
port_check = self._run_docker_cmd(
|
port_check = await self._run_docker_cmd(
|
||||||
["ps", "-q", "-f", f"publish={port}"], check=False
|
["ps", "-q", "-f", f"publish={port}"], check=False
|
||||||
)
|
)
|
||||||
if port_check.stdout.strip():
|
if port_check.stdout.strip():
|
||||||
@ -583,7 +603,7 @@ class DockerMixin(MCPMixin):
|
|||||||
label_args.extend(["-l", f"{k}={v}"])
|
label_args.extend(["-l", f"{k}={v}"])
|
||||||
|
|
||||||
# Start the container
|
# Start the container
|
||||||
run_result = self._run_docker_cmd(
|
run_result = await self._run_docker_cmd(
|
||||||
[
|
[
|
||||||
"run",
|
"run",
|
||||||
"-d",
|
"-d",
|
||||||
@ -657,7 +677,7 @@ class DockerMixin(MCPMixin):
|
|||||||
container_port = None
|
container_port = None
|
||||||
container_session = None
|
container_session = None
|
||||||
try:
|
try:
|
||||||
inspect_result = self._run_docker_cmd(
|
inspect_result = await self._run_docker_cmd(
|
||||||
[
|
[
|
||||||
"inspect",
|
"inspect",
|
||||||
"--format",
|
"--format",
|
||||||
@ -683,10 +703,10 @@ class DockerMixin(MCPMixin):
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
# Stop the container
|
# Stop the container
|
||||||
self._run_docker_cmd(["stop", name_or_id])
|
await self._run_docker_cmd(["stop", name_or_id])
|
||||||
|
|
||||||
if remove:
|
if remove:
|
||||||
self._run_docker_cmd(["rm", name_or_id])
|
await self._run_docker_cmd(["rm", name_or_id])
|
||||||
|
|
||||||
# Release the port back to the pool
|
# Release the port back to the pool
|
||||||
if container_port:
|
if container_port:
|
||||||
@ -739,7 +759,7 @@ class DockerMixin(MCPMixin):
|
|||||||
args.append("-f")
|
args.append("-f")
|
||||||
args.append(name_or_id)
|
args.append(name_or_id)
|
||||||
|
|
||||||
result = self._run_docker_cmd(args)
|
result = await self._run_docker_cmd(args)
|
||||||
return {
|
return {
|
||||||
"success": True,
|
"success": True,
|
||||||
"container": name_or_id,
|
"container": name_or_id,
|
||||||
@ -803,7 +823,7 @@ class DockerMixin(MCPMixin):
|
|||||||
args.append(str(proj_path))
|
args.append(str(proj_path))
|
||||||
|
|
||||||
# Run build (this can take a while)
|
# Run build (this can take a while)
|
||||||
result = self._run_docker_cmd(args, capture=True)
|
result = await self._run_docker_cmd(args, capture=True)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"success": True,
|
"success": True,
|
||||||
@ -1071,12 +1091,12 @@ class DockerMixin(MCPMixin):
|
|||||||
}
|
}
|
||||||
|
|
||||||
# Find orphaned containers
|
# Find orphaned containers
|
||||||
containers = self._find_containers_by_label(session_only=session_only)
|
containers = await self._find_containers_by_label(session_only=session_only)
|
||||||
|
|
||||||
for container in containers:
|
for container in containers:
|
||||||
# Check if container is old enough to be considered orphaned
|
# Check if container is old enough to be considered orphaned
|
||||||
try:
|
try:
|
||||||
inspect_result = self._run_docker_cmd(
|
inspect_result = await self._run_docker_cmd(
|
||||||
["inspect", "--format", "{{index .Config.Labels \"" + self.LABEL_PREFIX + ".started\"}}", container["id"]],
|
["inspect", "--format", "{{index .Config.Labels \"" + self.LABEL_PREFIX + ".started\"}}", container["id"]],
|
||||||
check=False,
|
check=False,
|
||||||
)
|
)
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user