Skip to content

test: Add failing tests for LDClient.close() releasing all components - #498

Open
aviadr1 wants to merge 1 commit into
launchdarkly:mainfrom
aviadr1:test/close-releases-all-components
Open

test: Add failing tests for LDClient.close() releasing all components#498
aviadr1 wants to merge 1 commit into
launchdarkly:mainfrom
aviadr1:test/close-releases-all-components

Conversation

@aviadr1

@aviadr1 aviadr1 commented Aug 15, 2026

Copy link
Copy Markdown

Failing tests for #495. Tests only — no fix.

Found while investigating #493, a production incident where LDClient.close() hung and left worker pods alive for hours to days, each holding a concurrency slot. Auditing the shutdown path afterwards turned up two further defects in close() itself. I have not observed these two in our own production, but both reproduce reliably.

What the tests pin

close() is documented as "Releases all threads and network connections used by the LaunchDarkly client". Two ways it does not:

test_close_releases_every_component_even_if_one_raisesclient.py:355-358 calls the event processor, data system and big segment store manager in sequence with no error handling. If the first raises, the other two are never stopped. Two of the three reach code the SDK does not control: the eventsource client, and the application's own BigSegmentStore implementation. The caller is left with leaked threads and connections from a client it believes is closed.

test_close_is_idempotent — there is no closed-flag, so a second close() re-runs the whole sequence. The recorded shutdown log is ['update_processor', 'big_segment_store', 'update_processor', 'big_segment_store'] — the application's own store is stopped twice, and under FDv2 store.close() is called twice.

Why xfail

Both are marked @pytest.mark.xfail(strict=True) so CI stays green while the defect is documented in the suite. Because it is strict, the moment the behaviour is fixed these turn into failures telling you to drop the marker.

To see the actual failures:

uv run pytest ldclient/testing/test_ldclient_shutdown.py --runxfail

Why no fix

Whether close() should still raise after releasing everything is a semantics decision for the SDK team, so the tests pin the leak rather than the exception behaviour. Happy to add the fix if you tell me which way you want it.


Note

Overview
Adds ldclient/testing/test_ldclient_shutdown.py — contract tests for sync LDClient.close() with no production code changes. Both cases are @pytest.mark.xfail(strict=True) so CI stays green until a fix lands.

test_close_releases_every_component_even_if_one_raises documents that client.py stops the event processor, data system, and big-segment manager in sequence with no error handling; if stop() on the first raises (e.g. eventsource or user BigSegmentStore), the others never run.

test_close_is_idempotent documents that without a closed flag, a second close() runs shutdown again (e.g. double stop() on the app’s big-segment store).

Uses injectable Recording* / FailingEventProcessor helpers and unreachable URIs so tests don’t need network.

Reviewed by Cursor Bugbot for commit a10751c. Bugbot is set up for automated code reviews on this repo. Configure here.

close() is documented as releasing all threads and network connections, but it
calls the event processor, the data system and the big segment store manager in
sequence with no error handling and no closed-flag.

Two failing tests (marked xfail strict, so they will start failing loudly once
the behaviour is fixed and the markers can be removed):

- test_close_releases_every_component_even_if_one_raises: if the first
  component's stop() raises, the data system and big segment store manager are
  never stopped. Two of the three reach code the SDK does not control - the
  eventsource client, and the application's own BigSegmentStore - so this is a
  realistic failure, not a contrived one. The caller is left with leaked threads
  and connections from a client it believes is closed.

- test_close_is_idempotent: with no closed-flag, a second close() re-runs the
  whole sequence, calling stop() twice on the application's BigSegmentStore and
  store.close() twice under FDv2. The recorded call log shows
  ['update_processor', 'big_segment_store', 'update_processor', 'big_segment_store'].

No fix is proposed here; these only pin the contract.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant