Skip to content

fix: sam local invoke leaks Docker container and temp directory on OOM #9184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Adityaj0 wants to merge 1 commit into
base: develop
Choose a base branch
from
+25 −4
Open

fix: sam local invoke leaks Docker container and temp directory on OOM #9184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions samcli/local/lambdafn/runtime.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,10 +356,13 @@ def _on_invoke_done(self, container):
container: Container
The current running container
"""
if container:
self._check_exit_state(container)
self._container_manager.stop(container)
self._clean_decompressed_paths()
try:
if container:
self._check_exit_state(container)
finally:
if container:
self._container_manager.stop(container)

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[RESOURCE_MANAGEMENT] The two cleanup steps run sequentially in the same finally block, so a failure in the first still skips the second — the same class of leak this PR fixes.

ContainerManager.stop() calls Container.stop() and Container.delete(), and both re-raise docker.errors.APIError unless the message matches the "removal of container ... is already in progress" special case (see samcli/local/docker/container.py). If that raises, self._clean_decompressed_paths() never runs, leaving the unzipped-code temp directory behind. It also replaces the in-flight ContainerFailureError, so an OOM'd invocation would surface an opaque Docker API error instead of "Container invocation failed due to maximum memory usage".

Nesting the cleanup makes both steps independent and preserves the original exception:

try:
   if container:
       self._check_exit_state(container)
finally:
   try:
       if container:
           self._container_manager.stop(container)
   finally:
       self._clean_decompressed_paths()

self._clean_decompressed_paths()

def _check_exit_state(self, container: Container):
"""
Expand Down
18 changes: 18 additions & 0 deletions tests/unit/local/lambdafn/test_runtime.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2014,6 +2014,24 @@ def test_on_invoke_done_with_none_container_only_cleans_paths(self):
# Verify cleanup was called
self.runtime._clean_decompressed_paths.assert_called_once()

def test_on_invoke_done_stops_container_and_cleans_paths_even_when_check_exit_state_raises(self):
"""Regression test: when the container was OOM-killed, _check_exit_state raises
ContainerFailureError. The container must still be stopped and the decompressed
code path must still be cleaned up, not skipped by the propagating exception.
"""
from samcli.local.docker.exceptions import ContainerFailureError

container = Mock()

self.runtime._check_exit_state = Mock(side_effect=ContainerFailureError("out of memory"))
self.runtime._clean_decompressed_paths = Mock()

with self.assertRaises(ContainerFailureError):
self.runtime._on_invoke_done(container)

self.manager_mock.stop.assert_called_once_with(container)
self.runtime._clean_decompressed_paths.assert_called_once()


class TestWarmLambdaRuntime_create_container_branch(TestCase):
"""Test WarmLambdaRuntime.create method container branch - lines 470->473"""
Expand Down