Skip to content

Commit 3e5cfca

Browse files
committed
Add streaming docs, changelog, and a leak benchmark
Document that streaming iterators now close their socket/fd on early break, exception, .close(), or GC. Add benchmarks/stream_leak.py, a daemon-free reproduction of #2766: opening many streams and stopping each after one chunk leaks every socket on the pre-fix generators and none on the current code. Signed-off-by: ykstorm <balveer767@gmail.com>
1 parent bbca190 commit 3e5cfca

5 files changed

Lines changed: 263 additions & 0 deletions

File tree

‎benchmarks/README.md‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Benchmarks
2+
3+
Small, self-contained scripts that demonstrate the behaviour of changes in
4+
this repository. They do not require a running Docker daemon.
5+
6+
## `stream_leak.py`
7+
8+
Reproduces the streaming socket/file-descriptor leak from
9+
[#2766](https://github.com/docker/docker-py/issues/2766) and shows that the
10+
`try/finally` cleanup added to the streaming generators fixes it.
11+
12+
It starts a local HTTP server that streams a chunked response indefinitely
13+
(mimicking `container.logs(stream=True, follow=True)`), then repeatedly opens a
14+
stream, reads a single chunk, and abandons the iterator — exactly the pattern
15+
that leaked before this change. After each batch it counts the process's open
16+
TCP connections.
17+
18+
```console
19+
$ python benchmarks/stream_leak.py --iterations 200
20+
opening 200 streams, reading one chunk, then stopping each early
21+
22+
impl streams sockets leaked ESTABLISHED conns
23+
------------------------------------------------------
24+
old 200 200 200
25+
fixed 200 0 0
26+
27+
PASS: old leaks all 200 sockets on early stop; fixed closes every one.
28+
```
29+
30+
Requires `psutil` (already a transitive test dependency). The `old` row uses a
31+
generator with no cleanup (the pre-fix behaviour); the `fixed` row uses the same
32+
generator wrapped in `try/finally: response.close()`, mirroring
33+
`APIClient._stream_raw_result`. `sockets leaked` is counted client-side
34+
(`http.client` responses still open); `ESTABLISHED conns` is the corroborating
35+
count from `psutil`.

‎benchmarks/stream_leak.py‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
#!/usr/bin/env python
2+
"""Demonstrate the streaming socket/fd leak from docker/docker-py#2766.
3+
4+
The streaming helpers in ``docker.api.client`` hand the caller a generator that
5+
reads from a long-lived socket. Before this change, abandoning that generator
6+
early (``break``, an exception, or simply dropping the reference) never closed
7+
the underlying response, so the socket/fd leaked.
8+
9+
This script reproduces the leak without a Docker daemon, at the raw socket
10+
level so the result does not depend on connection pooling or garbage-collection
11+
timing. It serves an endless chunked HTTP response from a local thread, then
12+
repeatedly:
13+
14+
1. opens a streaming connection,
15+
2. reads a single chunk,
16+
3. stops the iterator early (``generator.close()``),
17+
18+
while holding every connection open for the whole run.
19+
20+
``leaky`` is a generator with no cleanup (the pre-fix behaviour). ``fixed``
21+
wraps the same loop in ``try/finally: connection.close()`` -- the analogue of
22+
``response.close()`` that ``APIClient._stream_raw_result`` now performs. Each
23+
``connection.close()`` here closes exactly one socket, the same way
24+
``requests.Response.close()`` releases the docker daemon socket.
25+
26+
Usage:
27+
python benchmarks/stream_leak.py [--iterations N]
28+
"""
29+
import argparse
30+
import http.client
31+
import threading
32+
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
33+
34+
import psutil
35+
36+
37+
class _StreamingHandler(BaseHTTPRequestHandler):
38+
protocol_version = 'HTTP/1.1' # keep-alive: socket stays open until closed
39+
40+
# Stream chunks forever so the connection stays open until the client
41+
# closes it -- the follow=True case.
42+
def do_GET(self):
43+
self.send_response(200)
44+
self.send_header('Content-Type', 'application/octet-stream')
45+
self.send_header('Transfer-Encoding', 'chunked')
46+
self.end_headers()
47+
try:
48+
while True:
49+
payload = b'log line\n'
50+
self.wfile.write(
51+
f'{len(payload):x}\r\n'.encode() + payload + b'\r\n')
52+
self.wfile.flush()
53+
except (BrokenPipeError, ConnectionResetError, OSError):
54+
pass # client went away -- the whole point of the benchmark
55+
56+
def handle(self):
57+
try:
58+
super().handle()
59+
except (ConnectionError, OSError):
60+
pass # client closed mid-stream -- expected here
61+
62+
def log_message(self, *args):
63+
pass # keep the benchmark output clean
64+
65+
66+
def _start_server():
67+
server = ThreadingHTTPServer(('127.0.0.1', 0), _StreamingHandler)
68+
thread = threading.Thread(target=server.serve_forever, daemon=True)
69+
thread.start()
70+
return server
71+
72+
73+
class Stream:
74+
"""Minimal stand-in for a streaming docker response over a raw socket.
75+
76+
``close()`` releases the socket, mirroring ``requests.Response.close()``.
77+
"""
78+
79+
def __init__(self, host, port):
80+
self._conn = http.client.HTTPConnection(host, port)
81+
self._conn.request('GET', '/')
82+
self._resp = self._conn.getresponse()
83+
84+
def read(self, n=16):
85+
return self._resp.read(n)
86+
87+
def close(self):
88+
# Closing the response releases the socket -- the same call
89+
# APIClient now makes in its streaming generators' finally block.
90+
self._resp.close()
91+
self._conn.close()
92+
93+
@property
94+
def open(self):
95+
# http.client moves the socket into the response object, so check the
96+
# response rather than conn.sock. close() sets isclosed() True.
97+
return not self._resp.isclosed()
98+
99+
100+
def leaky(stream):
101+
"""Pre-fix behaviour: yields chunks, never closes the socket."""
102+
while True:
103+
data = stream.read()
104+
if not data:
105+
break
106+
yield data
107+
108+
109+
def fixed(stream):
110+
"""Post-fix behaviour, mirroring APIClient._stream_raw_result."""
111+
try:
112+
while True:
113+
data = stream.read()
114+
if not data:
115+
break
116+
yield data
117+
finally:
118+
stream.close()
119+
120+
121+
def established_to(proc, port):
122+
try:
123+
return sum(
124+
1 for c in proc.net_connections(kind='tcp')
125+
if c.raddr and c.raddr.port == port and c.status == 'ESTABLISHED'
126+
)
127+
except (psutil.AccessDenied, NotImplementedError):
128+
return -1
129+
130+
131+
def run(make_stream, host, port, iterations, proc):
132+
streams = []
133+
generators = []
134+
for _ in range(iterations):
135+
stream = Stream(host, port)
136+
gen = make_stream(stream)
137+
next(gen) # read a single chunk
138+
gen.close() # consumer stops early -> GeneratorExit
139+
streams.append(stream)
140+
generators.append(gen)
141+
142+
leaked = sum(1 for s in streams if s.open)
143+
established = established_to(proc, port)
144+
145+
for s in streams: # tidy up before the next run
146+
s.close()
147+
return leaked, established
148+
149+
150+
def main():
151+
parser = argparse.ArgumentParser(description=__doc__)
152+
parser.add_argument('--iterations', type=int, default=200,
153+
help='streams opened and abandoned per run')
154+
args = parser.parse_args()
155+
156+
server = _start_server()
157+
host, port = server.server_address
158+
proc = psutil.Process()
159+
160+
print(f'opening {args.iterations} streams, reading one chunk, then '
161+
f'stopping each early\n')
162+
header = (f'{"impl":<8}{"streams":>10}{"sockets leaked":>16}'
163+
f'{"ESTABLISHED conns":>20}')
164+
print(header)
165+
print('-' * len(header))
166+
167+
results = {}
168+
for name, fn in (('old', leaky), ('fixed', fixed)):
169+
leaked, established = run(fn, host, port, args.iterations, proc)
170+
results[name] = leaked
171+
print(f'{name:<8}{args.iterations:>10}{leaked:>16}{established:>20}')
172+
173+
server.shutdown()
174+
print()
175+
if results['old'] == args.iterations and results['fixed'] == 0:
176+
print(f'PASS: old leaks all {args.iterations} sockets on early stop; '
177+
f'fixed closes every one.')
178+
else:
179+
print('NOTE: compare the "sockets leaked" column for the two impls.')
180+
181+
182+
if __name__ == '__main__':
183+
main()

‎docs/change-log.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,16 @@
11
Changelog
22
==========
33

4+
Unreleased
5+
----------
6+
### Bugfixes
7+
- Fixed a socket/file-descriptor leak in streaming endpoints (`logs(stream=True)`,
8+
`events`, `attach`, `stats`, raw exec streams) when the consumer stopped
9+
iterating early, raised, or dropped the iterator without closing it
10+
([#2766](https://github.com/docker/docker-py/issues/2766)). The underlying
11+
response is now closed on early break, exception, `.close()`, or garbage
12+
collection.
13+
414
7.1.0
515
-----
616
### Upgrade Notes

‎docs/index.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ That's just a taste of what you can do with the Docker SDK for Python. For more,
9191
swarm
9292
volumes
9393
api
94+
streams
9495
tls
9596
user_guides/index
9697
change-log

‎docs/streams.rst‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
Streaming endpoints
2+
===================
3+
4+
Several SDK methods can stream data from the Docker daemon, for example
5+
``container.logs(stream=True)``, ``client.events()``, ``container.stats()``,
6+
``container.attach(stream=True)`` and ``client.api.build()``. These return
7+
iterators backed by an open socket to the daemon.
8+
9+
The SDK closes the underlying socket and file descriptor automatically when:
10+
11+
* the iterator is fully consumed,
12+
* you ``break`` out of the loop early or an exception is raised,
13+
* you call ``.close()`` on the iterator (where supported), or
14+
* the iterator is garbage collected.
15+
16+
You no longer need to drain a stream to completion just to avoid leaking a file
17+
descriptor. Consume what you need and let the iterator go out of scope:
18+
19+
.. code-block:: python
20+
21+
for line in container.logs(stream=True):
22+
if done(line):
23+
break # the socket is closed for you
24+
25+
The ``benchmarks/stream_leak.py`` script in the repository reproduces the leak
26+
this fixed (`#2766 <https://github.com/docker/docker-py/issues/2766>`_) without
27+
a daemon. Opening 200 streams and stopping each after a single chunk, the
28+
pre-fix generators leak every socket, while the current code closes all of
29+
them::
30+
31+
impl streams sockets leaked ESTABLISHED conns
32+
------------------------------------------------------
33+
old 200 200 200
34+
fixed 200 0 0

0 commit comments

Comments
 (0)