Skip to content

[ZEPPELIN-6539] Fix DockerClient / container resource leaks on interpreter failure paths#5302

Open
hyunw9 wants to merge 3 commits into
apache:masterfrom
hyunw9:ZEPPELIN-6539
Open

[ZEPPELIN-6539] Fix DockerClient / container resource leaks on interpreter failure paths#5302
hyunw9 wants to merge 3 commits into
apache:masterfrom
hyunw9:ZEPPELIN-6539

Conversation

@hyunw9

@hyunw9 hyunw9 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

What is this PR for?

The Docker interpreter launcher (DockerInterpreterProcess) can leak resources on error paths when interpreters are started/stopped repeatedly by notebook workloads.

  1. stop() may leak the DockerClient. docker.close() was called after the try/catch that kills/removes the container. If killContainer/removeContainer throws an unexpected (unchecked) exception, it is not caught and docker.close() is skipped, leaking the underlying HTTP socket / file descriptors.
  2. start() may leave an orphaned container. start() runs pull → create → start → copy files → exec → wait-for-register. If preparation fails after the container is started (e.g. copyRunFileToContainer / execInContainer), the running container is not cleaned up

What type of PR is it?

Bug Fix

Todos

  • Fix stop() to always close the DockerClient
  • Fix start() to clean up a started container on preparation failure
  • Add unit tests (Mockito interaction tests)

What is the Jira issue?

How should this be tested?

Automated unit tests added in DockerInterpreterProcessTest (DockerClient is mocked):

  • stop_alwaysClosesDockerClient_evenWhenKillContainerFails
  • start_removesContainer_whenContainerPreparationFails

Screenshots (if appropriate)

Questions:

  • Does the license files need to update? - No
  • Is there breaking changes for older versions? - No
  • Does this needs documentation? - No

// Best-effort removal of a container that was (partially) created during start().
private void cleanupContainerQuietly() {
try {
docker.killContainer(containerName);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion (non-blocking): In cleanupContainerQuietly(), killContainer and removeContainer share one try, so if killContainer throws (e.g. the container was created but never started, so it isn't running), removeContainer is skipped and the container is still leaked. Could we run them independently so removeContainer always fires?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants