Skip to content

Commit 4e6f1bf

Browse files
committed
refactor: Simplify async client close() teardown
Remove the close() timeout: no other LaunchDarkly SDK bounds close, and without a timeout the whole teardown always runs, so nothing leaks. Merge the failed-start cleanup into _close_components so both close() and a failed start() share one idempotent teardown, and mark a failed start closed so a later close() is a no-op.
1 parent 3958058 commit 4e6f1bf

2 files changed

Lines changed: 28 additions & 44 deletions

File tree

ldclient/async_client.py

Lines changed: 17 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -148,31 +148,13 @@ async def start(self, start_wait: float = 5.0) -> None:
148148
try:
149149
await self.__start_up(start_wait)
150150
except BaseException:
151-
# Catch BaseException so a cancelled start (CancelledError) also
152-
# releases the components __start_up began; re-raise propagates it.
153-
await self._cleanup_partial_start()
151+
# Catch BaseException, not Exception, so a cancelled start
152+
# (CancelledError) also tears down what __start_up began.
153+
self._closed = True
154+
await self._close_components()
154155
raise
155156

156-
async def _cleanup_partial_start(self):
157-
"""Release any resources that were partially created during a failed
158-
__start_up."""
159-
for name, stop in (
160-
("data system", self._data_system.stop),
161-
("big-segment store manager", self.__big_segment_store_manager.stop),
162-
("event processor", self._event_processor.stop),
163-
):
164-
try:
165-
await stop()
166-
except Exception as e:
167-
log.warning("Error stopping %s during failed start: %s", name, e)
168-
if self._session is not None:
169-
try:
170-
await self._session.close()
171-
except Exception as e:
172-
log.warning("Error closing HTTP session during failed start: %s", e)
173-
self._session = None
174-
175-
async def close(self, close_timeout: float = 2.0) -> None:
157+
async def close(self) -> None:
176158
"""Shut down the client and release all resources.
177159
178160
Safe to call multiple times — subsequent calls are no-ops.
@@ -184,29 +166,26 @@ async def close(self, close_timeout: float = 2.0) -> None:
184166

185167
# A client that was never started has nothing running to release.
186168
if self._started:
187-
try:
188-
await asyncio.wait_for(self._close_components(), timeout=close_timeout)
189-
except asyncio.TimeoutError:
190-
log.warning("Timed out closing AsyncLDClient components")
191-
except Exception as e:
192-
log.warning("Error closing AsyncLDClient components: %s", e)
193-
194-
if self._session is not None:
195-
try:
196-
await self._session.close()
197-
except Exception as e:
198-
log.warning("Error closing HTTP session: %s", e)
169+
await self._close_components()
199170

200171
async def _close_components(self):
201-
"""Releases the threads and network connections used by the SDK
202-
components. The public :meth:`close` wraps this with a timeout."""
172+
"""Stop the SDK components and release the shared HTTP session."""
203173
log.info("Closing LaunchDarkly client..")
174+
204175
await self._data_system.stop()
205176
await self.__big_segment_store_manager.stop()
206177

207-
# The event processor is last because it may still be sending events that were generated by the other components.
178+
# The event processor is last because it may still be sending events the
179+
# other components generated.
208180
await self._event_processor.stop()
209181

182+
if self._session is not None:
183+
try:
184+
await self._session.close()
185+
except Exception as e:
186+
log.warning("Error closing HTTP session: %s", e)
187+
self._session = None
188+
210189
async def __start_up(self, start_wait: float):
211190
environment_metadata = get_environment_metadata(self._config, "python-server-sdk-async")
212191
plugin_hooks = get_plugin_hooks(self._config.plugins, environment_metadata)

ldclient/testing/test_async_client.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ async def test_flush_delegates_to_event_processor():
127127
# Replace the event processor with a mock that tracks flush calls
128128
mock_ep = MagicMock()
129129
mock_ep.flush = MagicMock(return_value=None)
130+
mock_ep.stop = AsyncMock()
130131
client._event_processor = mock_ep
131132

132133
await client.flush()
@@ -143,6 +144,7 @@ async def test_flush_is_noop_when_offline():
143144

144145
mock_ep = MagicMock()
145146
mock_ep.flush = MagicMock(return_value=None)
147+
mock_ep.stop = AsyncMock()
146148
client._event_processor = mock_ep
147149

148150
await client.flush()
@@ -384,8 +386,9 @@ def boom(*args, **kwargs):
384386

385387
with pytest.raises(RuntimeError):
386388
await client.start()
387-
# A failed start marks the client started (spent); a retry is a no-op.
389+
# A failed start marks the client started (spent) and closed (torn down).
388390
assert client._started is True
391+
assert client._closed is True
389392

390393
# Retry does not re-run start-up (it would raise again if it did).
391394
await client.start()
@@ -398,14 +401,14 @@ async def test_start_cleans_up_and_propagates_on_cancellation(monkeypatch):
398401
client = AsyncLDClient(_offline_config())
399402

400403
cleaned = False
401-
original_cleanup = client._cleanup_partial_start
404+
original_cleanup = client._close_components
402405

403-
async def spy_cleanup():
406+
async def spy_cleanup(*args, **kwargs):
404407
nonlocal cleaned
405408
cleaned = True
406-
await original_cleanup()
409+
await original_cleanup(*args, **kwargs)
407410

408-
client._cleanup_partial_start = spy_cleanup
411+
client._close_components = spy_cleanup
409412

410413
def cancel(*args, **kwargs):
411414
raise asyncio.CancelledError()
@@ -416,9 +419,11 @@ def cancel(*args, **kwargs):
416419
await client.start()
417420

418421
# Cleanup ran (stopping the started components), the instance is spent
419-
# (single-shot via _started), and the CancelledError propagated.
422+
# (single-shot via _started), it is closed, and the CancelledError
423+
# propagated.
420424
assert cleaned is True
421425
assert client._started is True
426+
assert client._closed is True
422427

423428

424429
@pytest.mark.asyncio

0 commit comments

Comments
 (0)