Skip to content

Commit 8d0de4e

Browse files
committed
fix: Require urllib3 2.x and drop the 1.26 stream-close fallback
The connection closer now unconditionally calls resp.shutdown() then resp.close() (2.x APIs) to deterministically close the streaming socket; the urllib3 floor is raised to >=2.0.0 and the 1.26 release_conn fallback and its version-gated tests are removed.
1 parent c74bbb6 commit 8d0de4e

4 files changed

Lines changed: 17 additions & 52 deletions

File tree

ld_eventsource/http.py

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -115,31 +115,16 @@ def connect(self, last_event_id: Optional[str]) -> Tuple[Iterator[bytes], Callab
115115
stream = resp.stream(_CHUNK_SIZE)
116116

117117
def close():
118-
if hasattr(resp, "shutdown"):
119-
# urllib3 2.x: shutdown() wakes a reader blocked mid-read (SHUT_RD) so the
120-
# subsequent close() -- which calls self._fp.close() and would otherwise
121-
# deadlock on the reader's BufferedReader lock -- can proceed. close() then
122-
# sends the TCP FIN and releases the fd. Both are built-ins.
123-
try:
124-
resp.shutdown()
125-
except Exception:
126-
self.__logger.debug("Error interrupting stream via resp.shutdown()", exc_info=True)
127-
resp.close()
128-
else:
129-
# urllib3 1.26.x has no resp.shutdown(), and we deliberately do not reach
130-
# into private socket attributes to find and shut down the socket. Without a
131-
# way to wake a blocked reader first, resp.close() could deadlock on the
132-
# reader's BufferedReader lock, so we fall back to the original behavior of
133-
# releasing the connection back to the pool. This never hangs, but the socket
134-
# is not closed deterministically -- deterministic close requires urllib3 2.x.
135-
resp.release_conn()
118+
try:
119+
resp.shutdown()
120+
except Exception:
121+
self.__logger.debug("Error interrupting stream via resp.shutdown()", exc_info=True)
122+
resp.close()
136123

137124
return stream, close, response_headers
138125

139126
def close(self):
140127
if self.__should_close_pool:
141-
# Only clear a pool we created. On urllib3 2.x the active connection was already
142-
# closed by the connection closer (resp.close()); we do not iterate and close the
143-
# pool's connections ourselves, because doing so hangs on urllib3 1.26.x when a
144-
# reader is still blocked mid-read on a connection.
128+
# Only clear a pool we created; the active connection was already closed by the
129+
# connection closer (resp.close()), so clearing the pool is all that's left.
145130
self.__pool.clear()

ld_eventsource/testing/test_http_connect_strategy.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -256,19 +256,14 @@ def test_close_leaves_caller_supplied_pool_open():
256256

257257

258258
def test_close_clears_pool_it_created():
259-
# When the client creates its own pool (no pool supplied), close() clears that pool.
260-
# It must NOT iterate and close the individual connection pools itself: on urllib3
261-
# 1.26.x that hangs when a reader is still blocked mid-read on a connection. On
262-
# urllib3 2.x the active connection is already closed by the connection closer.
263-
connection_pool = mock.Mock()
259+
# When the client creates its own pool (no pool supplied), close() clears it. The active
260+
# connection is already closed by the connection closer (resp.close()), so clearing the
261+
# pool is all that's needed here.
264262
created_pool = mock.MagicMock()
265-
created_pool.pools.keys.return_value = ['poolkey']
266-
created_pool.pools.get.return_value = connection_pool
267263

268264
with mock.patch('ld_eventsource.http.PoolManager', return_value=created_pool):
269265
client = ConnectStrategy.http("http://test").create_client(logger())
270266

271267
client.close()
272268

273269
created_pool.clear.assert_called_once()
274-
connection_pool.close.assert_not_called()

ld_eventsource/testing/test_http_connect_strategy_with_sse_client.py

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
import time
33
from urllib.parse import parse_qsl
44

5-
import urllib3.response
6-
75
from ld_eventsource import *
86
from ld_eventsource.actions import *
97
from ld_eventsource.config import *
@@ -97,18 +95,10 @@ def test_sse_client_reconnects_after_interrupt():
9795
assert event2.data == 'data2'
9896

9997

100-
# urllib3 2.x exposes HTTPResponse.shutdown(), which lets the closer wake a reader that is
101-
# blocked mid-read before closing the connection. urllib3 1.26.x has no such built-in, and
102-
# under design U-prime we deliberately do not reach into private socket attributes to wake
103-
# the reader. So on 2.x the reader is woken; on 1.26.x it is not -- but in NEITHER case may
104-
# the stop call itself hang.
105-
_CAN_WAKE_READER = hasattr(urllib3.response.HTTPResponse, "shutdown")
106-
107-
10898
def _run_concurrent_stop_test(stop_method_name):
10999
# Exercises the concurrent shutdown pattern: a worker thread blocks mid-read on an idle
110100
# stream while another thread stops the client. The stop call must return within the
111-
# timeout on every urllib3 version (a hang here is the regression we guard against).
101+
# timeout (a hang here is the regression we guard against).
112102
with start_server() as server:
113103
with make_stream() as stream:
114104
server.for_path('/', stream)
@@ -146,18 +136,13 @@ def stopper():
146136
stopper_thread = threading.Thread(target=stopper, daemon=True)
147137
stopper_thread.start()
148138

149-
# Must never hang, on any urllib3 version.
139+
# Must never hang.
150140
assert stopped.wait(timeout=5), \
151141
"%s() deadlocked on a concurrently blocked reader" % stop_method_name
152142

153-
if _CAN_WAKE_READER:
154-
# urllib3 2.x: the closer's resp.shutdown() wakes the blocked reader.
155-
assert reader_woke.wait(timeout=5), \
156-
"%s() did not wake the concurrently blocked reader" % stop_method_name
157-
else:
158-
# urllib3 1.26.x: the reader is intentionally not woken (U-prime does not
159-
# touch private socket attrs); only the no-hang guarantee above applies.
160-
pass
143+
# The closer's resp.shutdown() wakes the blocked reader.
144+
assert reader_woke.wait(timeout=5), \
145+
"%s() did not wake the concurrently blocked reader" % stop_method_name
161146
finally:
162147
client.close()
163148

pyproject.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ classifiers = [
2222
"Topic :: Software Development :: Libraries",
2323
]
2424
dependencies = [
25-
"urllib3>=1.26.0,<3",
25+
"urllib3>=2.0.0,<3",
2626
]
2727

2828
[project.urls]
@@ -55,7 +55,7 @@ docs = [
5555
"pyrfc3339>=1.0",
5656
"jsonpickle>1.4.1",
5757
"semver>=2.7.9",
58-
"urllib3>=1.26.0",
58+
"urllib3>=2.0.0",
5959
"jinja2>=3.1.2",
6060
]
6161

0 commit comments

Comments
 (0)