Skip to content

Conversation

@Nitrooo
Copy link
Owner

@Nitrooo Nitrooo commented Jul 4, 2022

Requests with stream=True MUST be closed or else the connection will
never be returned to the connection pool. Both ContainerApiMixin.attach
and ExecApiMixin.exec_start were leaking in the stream=False case.
exec_start was modified to follow attach for the stream=True case as
that allows the caller to close the stream when done (untested).

Tested with:

# Test exec_run (stream=False) - observe one less leak
make integration-test-py3 file=models_containers_test.py' -k test_exec_run_success -vs -W error::ResourceWarning'
# Test exec_start (stream=True, fully reads from CancellableStream)
make integration-test-py3 file=api_exec_test.py' -k test_execute_command -vs -W error::ResourceWarning'

After this change, one resource leak is removed, the remaining resource
leaks occur because none of the tests call client.close().

Fixes docker#1293
(Regression from docker#1130)

Signed-off-by: Peter Wu [email protected]

Requests with stream=True MUST be closed or else the connection will
never be returned to the connection pool. Both ContainerApiMixin.attach
and ExecApiMixin.exec_start were leaking in the stream=False case.
exec_start was modified to follow attach for the stream=True case as
that allows the caller to close the stream when done (untested).

Tested with:

    # Test exec_run (stream=False) - observe one less leak
    make integration-test-py3 file=models_containers_test.py' -k test_exec_run_success -vs -W error::ResourceWarning'
    # Test exec_start (stream=True, fully reads from CancellableStream)
    make integration-test-py3 file=api_exec_test.py' -k test_execute_command -vs -W error::ResourceWarning'

After this change, one resource leak is removed, the remaining resource
leaks occur because none of the tests call client.close().

Fixes docker#1293
(Regression from docker#1130)

Signed-off-by: Peter Wu <[email protected]>
@Nitrooo Nitrooo merged commit cffe745 into Nitrooo:master Jul 4, 2022
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.

ResourceWarning: unclosed <socket.socket>

2 participants