diff --git a/src/ghydramcp/mixins/docker.py b/src/ghydramcp/mixins/docker.py index 7626e42..9dc1343 100644 --- a/src/ghydramcp/mixins/docker.py +++ b/src/ghydramcp/mixins/docker.py @@ -510,7 +510,6 @@ class DockerMixin(MCPMixin): async def docker_start( self, binary_path: str, - port: Optional[int] = None, memory: str = "2G", name: Optional[str] = None, ctx: Optional[Context] = None, @@ -519,15 +518,14 @@ class DockerMixin(MCPMixin): This creates a new Ghidra instance in Docker with the GhydraMCP plugin pre-installed. The binary will be imported and analyzed, - then the HTTP API will be available on the specified port. + then the HTTP API will be available. - If no port is specified, one will be automatically allocated from - the pool (8192-8199). Container names are auto-generated with the - session ID to ensure uniqueness across processes. + Ports are automatically allocated from the pool (8192-8199) to + prevent conflicts between concurrent sessions. Container names + are auto-generated with the session ID to ensure uniqueness. Args: binary_path: Path to the binary file to analyze - port: Port to expose the HTTP API (auto-allocated if not specified) memory: Max JVM heap memory (default: 2G) name: Container name (auto-generated if not specified) @@ -542,16 +540,13 @@ class DockerMixin(MCPMixin): if not binary_file.exists(): return {"error": f"Binary not found: {binary_path}"} - # Allocate port from pool if not specified - allocated_port = False + # Always allocate from pool to prevent conflicts between sessions + port = self._port_pool.allocate(self.session_id) if port is None: - port = self._port_pool.allocate(self.session_id) - if port is None: - return { - "error": "Port pool exhausted (8192-8199). Stop some containers first.", - "allocated_ports": self._port_pool.get_allocated_ports(), - } - allocated_port = True + return { + "error": "Port pool exhausted (8192-8199). Stop some containers first.", + "allocated_ports": self._port_pool.get_allocated_ports(), + } # Generate container name if not specified if name is None: @@ -566,8 +561,7 @@ class DockerMixin(MCPMixin): ["ps", "-a", "-q", "-f", f"name=^{name}$"], check=False ) if check_result.stdout.strip(): - if allocated_port: - self._port_pool.release(port) + self._port_pool.release(port) return { "error": f"Container '{name}' already exists. Stop it first with docker_stop." } @@ -577,8 +571,7 @@ class DockerMixin(MCPMixin): ["ps", "-q", "-f", f"publish={port}"], check=False ) if port_check.stdout.strip(): - if allocated_port: - self._port_pool.release(port) + self._port_pool.release(port) return { "error": f"Port {port} is already in use by another container" } @@ -616,7 +609,6 @@ class DockerMixin(MCPMixin): "port": port, "binary": str(binary_file), "memory": memory, - "allocated_port": allocated_port, } return { @@ -635,8 +627,7 @@ class DockerMixin(MCPMixin): } except subprocess.CalledProcessError as e: - if allocated_port: - self._port_pool.release(port) + self._port_pool.release(port) return {"error": f"Failed to start container: {e.stderr or e.stdout}"} @mcp_tool( @@ -899,7 +890,6 @@ class DockerMixin(MCPMixin): async def docker_auto_start( self, binary_path: str, - port: Optional[int] = None, wait: bool = True, timeout: float = 300.0, ctx: Optional[Context] = None, @@ -907,18 +897,16 @@ class DockerMixin(MCPMixin): """Automatically start a Docker container with intelligent port allocation. This is the main entry point for automatic Docker management: - 1. Checks if a Ghidra instance is already running (on specified or any pooled port) + 1. Checks if a Ghidra instance with the SAME binary is already running 2. If not, allocates a port from the pool and starts a new container 3. Optionally waits for the container to become healthy 4. Returns connection info for the instance - When port is not specified, the system will: - - First check all pooled ports (8192-8199) for an existing healthy instance - - If none found, allocate a new port from the pool + Ports are auto-allocated from the pool (8192-8199) to prevent + conflicts between concurrent sessions. Args: binary_path: Path to the binary to analyze - port: Specific port for the HTTP API (auto-allocated if not specified) wait: Wait for container to be ready (default: True) timeout: Max wait time in seconds (default: 300) @@ -935,31 +923,18 @@ class DockerMixin(MCPMixin): return False return os.path.basename(health_program) == requested_name - # If port is specified, check that specific port - if port is not None: - health = await self.docker_health(port=port, ctx=ctx) + # Check all pooled ports for an instance with the SAME binary + for check_port in range(PORT_POOL_START, PORT_POOL_END + 1): + health = await self.docker_health(port=check_port, timeout=1.0, ctx=ctx) if health.get("healthy") and _is_same_binary(health.get("program", "")): return { "source": "existing", "session_id": self.session_id, - "port": port, - "api_url": f"http://localhost:{port}/", + "port": check_port, + "api_url": f"http://localhost:{check_port}/", "program": health.get("program"), - "message": "Using existing Ghidra instance", + "message": f"Found existing Ghidra instance on port {check_port}", } - else: - # Check all pooled ports for an instance with the SAME binary - for check_port in range(PORT_POOL_START, PORT_POOL_END + 1): - health = await self.docker_health(port=check_port, timeout=1.0, ctx=ctx) - if health.get("healthy") and _is_same_binary(health.get("program", "")): - return { - "source": "existing", - "session_id": self.session_id, - "port": check_port, - "api_url": f"http://localhost:{check_port}/", - "program": health.get("program"), - "message": f"Found existing Ghidra instance on port {check_port}", - } # Check if Docker is available status = await self.docker_status(ctx=ctx) @@ -977,9 +952,9 @@ class DockerMixin(MCPMixin): ) } - # Start a new container (port will be auto-allocated if not specified) + # Start a new container (port auto-allocated from pool) start_result = await self.docker_start( - binary_path=binary_path, port=port, ctx=ctx + binary_path=binary_path, ctx=ctx ) if not start_result.get("success"):