Skip to content

Commit a922578

Browse files
committed
fix(wait-strategy): close HTTPError in HttpWaitStrategy to avoid fd leak
Closes #1115
1 parent 8d81011 commit a922578

2 files changed

Lines changed: 54 additions & 7 deletions

File tree

‎src/testcontainers/core/wait_strategies.py‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -427,13 +427,19 @@ def _try_http_request(self, url: str, headers: dict[str, str], ssl_context: Any)
427427

428428
def _handle_http_error(self, error: Union[URLError, HTTPError]) -> bool:
429429
"""Handle HTTP errors and return True if error is acceptable."""
430-
if isinstance(error, HTTPError) and (
431-
error.code in self._status_codes
432-
or (self._status_code_predicate and self._status_code_predicate(error.code))
433-
):
434-
return True
435-
logger.debug(f"HTTP request failed: {error!s}")
436-
return False
430+
try:
431+
if isinstance(error, HTTPError) and (
432+
error.code in self._status_codes
433+
or (self._status_code_predicate and self._status_code_predicate(error.code))
434+
):
435+
return True
436+
logger.debug(f"HTTP request failed: {error!s}")
437+
return False
438+
finally:
439+
# HTTPError wraps the response body in a temporary file: always close it
440+
# to avoid leaking file descriptors (see #1115).
441+
if isinstance(error, HTTPError):
442+
error.close()
437443

438444

439445
class HealthcheckWaitStrategy(WaitStrategy):

‎tests/core/test_wait_strategies.py‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import itertools
22
import logging
33
import re
4+
import threading
45
import time
56
from datetime import timedelta
7+
from http.server import BaseHTTPRequestHandler, HTTPServer
68
from unittest.mock import Mock, patch
9+
from urllib.error import HTTPError
710

811
import pytest
912

@@ -361,6 +364,44 @@ def test_from_url(self, url, expected_port, expected_path, expected_tls):
361364
assert strategy._path == expected_path
362365
assert strategy._tls is expected_tls
363366

367+
@pytest.mark.parametrize(
368+
"status_code,expected",
369+
[(503, False), (200, True)],
370+
ids=["unexpected_status", "expected_status"],
371+
)
372+
def test_handle_http_error_always_closes_error(self, status_code, expected):
373+
# Regression test for #1115: HTTPError wraps the response fp and must be closed.
374+
error = Mock(spec=HTTPError)
375+
error.code = status_code
376+
strategy = HttpWaitStrategy(8080, "/health").for_status_code(200)
377+
assert strategy._handle_http_error(error) is expected
378+
error.close.assert_called_once_with()
379+
380+
def test_try_http_request_unexpected_status(self):
381+
# End-to-end check for #1115: 503 against an expectation of 200 returns False.
382+
class _FailHandler(BaseHTTPRequestHandler):
383+
def do_GET(self):
384+
body = b"error"
385+
self.send_response(503)
386+
self.send_header("Content-Length", str(len(body)))
387+
self.end_headers()
388+
self.wfile.write(body)
389+
390+
def log_message(self, *args):
391+
pass
392+
393+
server = HTTPServer(("127.0.0.1", 0), _FailHandler)
394+
thread = threading.Thread(target=server.serve_forever, daemon=True)
395+
thread.start()
396+
try:
397+
port = server.server_address[1]
398+
url = f"http://127.0.0.1:{port}/health"
399+
assert HttpWaitStrategy(port, "/health").for_status_code(200)._try_http_request(url, {}, None) is False
400+
assert HttpWaitStrategy(port, "/health").for_status_code(503)._try_http_request(url, {}, None) is True
401+
finally:
402+
server.shutdown()
403+
thread.join(timeout=5)
404+
364405

365406
class TestHealthcheckWaitStrategy:
366407
"""Test the HealthcheckWaitStrategy class."""

0 commit comments

Comments
 (0)