fix: ensure Docker containers are always removed when closing sessions (#35)

When closing sessions with already-stopped containers, the stop/kill
operation would raise an exception, preventing container.remove() from
being called. This left stopped containers in Docker even though they
were removed from cubbi's session tracking.

The fix wraps stop/kill operations in their own try-except block,
allowing the code to always reach container.remove() regardless of
whether the container was already stopped.
This commit is contained in:
2025-10-06 16:40:50 -06:00
committed by GitHub
parent 3795de1484
commit b788f3f52e

View File

@@ -888,35 +888,39 @@ class ContainerManager:
return False return False
try: try:
# First, close the main session container
container = self.client.containers.get(session.container_id) container = self.client.containers.get(session.container_id)
if kill:
container.kill() try:
else: if kill:
container.stop() container.kill()
else:
container.stop()
except DockerException:
pass
container.remove() container.remove()
# Check for and close any associated network-filter container
network_filter_name = f"cubbi-network-filter-{session.id}" network_filter_name = f"cubbi-network-filter-{session.id}"
try: try:
network_filter_container = self.client.containers.get( network_filter_container = self.client.containers.get(
network_filter_name network_filter_name
) )
logger.info(f"Stopping network-filter container {network_filter_name}") logger.info(f"Stopping network-filter container {network_filter_name}")
if kill: try:
network_filter_container.kill() if kill:
else: network_filter_container.kill()
network_filter_container.stop() else:
network_filter_container.stop()
except DockerException:
pass
network_filter_container.remove() network_filter_container.remove()
except DockerException: except DockerException:
# Network-filter container might not exist, which is fine
pass pass
self.session_manager.remove_session(session.id) self.session_manager.remove_session(session.id)
return True return True
except DockerException as e: except DockerException as e:
error_message = str(e).lower() error_message = str(e).lower()
# If container is not running or already removed, still remove it from session list
if ( if (
"is not running" in error_message "is not running" in error_message
or "no such container" in error_message or "no such container" in error_message
@@ -951,39 +955,41 @@ class ContainerManager:
# No need for session status as we receive it via callback # No need for session status as we receive it via callback
# Define a wrapper to track progress
def close_with_progress(session): def close_with_progress(session):
if not session.container_id: if not session.container_id:
return False return False
try: try:
container = self.client.containers.get(session.container_id) container = self.client.containers.get(session.container_id)
# Stop and remove container
if kill: try:
container.kill() if kill:
else: container.kill()
container.stop() else:
container.stop()
except DockerException:
pass
container.remove() container.remove()
# Check for and close any associated network-filter container
network_filter_name = f"cubbi-network-filter-{session.id}" network_filter_name = f"cubbi-network-filter-{session.id}"
try: try:
network_filter_container = self.client.containers.get( network_filter_container = self.client.containers.get(
network_filter_name network_filter_name
) )
if kill: try:
network_filter_container.kill() if kill:
else: network_filter_container.kill()
network_filter_container.stop() else:
network_filter_container.stop()
except DockerException:
pass
network_filter_container.remove() network_filter_container.remove()
except DockerException: except DockerException:
# Network-filter container might not exist, which is fine
pass pass
# Remove from session storage
self.session_manager.remove_session(session.id) self.session_manager.remove_session(session.id)
# Notify about completion
if progress_callback: if progress_callback:
progress_callback( progress_callback(
session.id, session.id,
@@ -994,7 +1000,6 @@ class ContainerManager:
return True return True
except DockerException as e: except DockerException as e:
error_message = str(e).lower() error_message = str(e).lower()
# If container is not running or already removed, still remove it from session list
if ( if (
"is not running" in error_message "is not running" in error_message
or "no such container" in error_message or "no such container" in error_message