Skip to content

Commit

Permalink
Fix stuck conversion processes
Browse files Browse the repository at this point in the history
Gracefully terminate certain conversion processes that may get stuck
when writing lots of data to stdout. Also, handle a race condition when
a conversion process terminates slightly after the associated container.

Fixes #791
  • Loading branch information
apyrgio committed May 9, 2024
1 parent 0557e34 commit ff25fa3
Showing 1 changed file with 32 additions and 8 deletions.
40 changes: 32 additions & 8 deletions dangerzone/isolation_provider/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,18 +228,16 @@ def kill_container(self, name: str) -> None:
container_runtime = self.get_runtime()
cmd = [container_runtime, "kill", name]
try:
# We do not check the exit code of the process here, since the container may
# have stopped right before invoking this command. In that case, the
# command's output will contain some error messages, so we capture them in
# order to silence them.
subprocess.run(
cmd,
capture_output=True,
check=True,
startupinfo=get_subprocess_startupinfo(),
cmd, capture_output=True, startupinfo=get_subprocess_startupinfo()
)
except subprocess.CalledProcessError as e:
log.warning(f"Failed to kill container {name}: {str(e)}")
log.warning(f"Output from the kill command: {e.output}")
except Exception as e:
log.exception(
f"Unexpected error occurred while killing container {name}: {str(e)}"
f"Unexpected error occurred while killing container '{name}': {str(e)}"
)

def pixels_to_pdf(
Expand Down Expand Up @@ -303,7 +301,33 @@ def start_doc_to_pixels_proc(self, document: Document) -> subprocess.Popen:
def terminate_doc_to_pixels_proc(
self, document: Document, p: subprocess.Popen
) -> None:
# There are two steps to gracefully terminate a conversion process:
# 1. Kill the container, and check that it has exited.
# 2. Gracefully terminate the conversion process, in case it's stuck on I/O
#
# See also https://github.com/freedomofpress/dangerzone/issues/791
self.kill_container(self.doc_to_pixels_container_name(document))
p.terminate()

def ensure_stop_doc_to_pixels_proc( # type: ignore [no-untyped-def]
self, document: Document, *args, **kwargs
) -> None:
super().ensure_stop_doc_to_pixels_proc(document, *args, **kwargs)

# Check if the container no longer exists, either because we successfully killed
# it, or because it exited on its own. We operate under the assumption that
# after a podman kill / docker kill invocation, this will likely be the case,
# else the container runtime (Docker/Podman) has experienced a problem, and we
# should report it.
container_runtime = self.get_runtime()
name = self.doc_to_pixels_container_name(document)
all_containers = subprocess.run(
[container_runtime, "ps", "-a"],
capture_output=True,
startupinfo=get_subprocess_startupinfo(),
)
if name in all_containers.stdout.decode():
log.warning(f"Container '{name}' did not stop gracefully")

def get_max_parallel_conversions(self) -> int:
# FIXME hardcoded 1 until length conversions are better handled
Expand Down

0 comments on commit ff25fa3

Please sign in to comment.