Skip to content

fix: Background threads leaking across tests#229

Merged
khvn26 merged 3 commits into
mainfrom
fix/leaked-threads-pytest9
Jun 26, 2026
Merged

fix: Background threads leaking across tests#229
khvn26 merged 3 commits into
mainfrom
fix/leaked-threads-pytest9

Conversation

@khvn26

@khvn26 khvn26 commented Jun 26, 2026

Copy link
Copy Markdown
Member

Background worker threads could outlive the test that created them, then race with pyfakefs in a later test. This never surfaced until #228 because pytest 9 turns the resulting unhandled thread exceptions into test errors.

The changes here are version-agnostic and pass on both pytest 7.4.4 (current main) and pytest 9.0.3.

Testing:

  • Reproduced both failures deterministically against pytest 9.0.3 / pyfakefs 5.10.2, and confirmed they disappear with these changes (suite run repeatedly, including under -W error).
  • Full suite green on Python 3.10, 3.12 and 3.13.

The polling and streaming worker threads could outlive the test that
created them. Once leaked, they raced with pyfakefs (which is not
thread-safe) during another test's filesystem patching, corrupting it,
and pytest 9 turns the resulting unhandled thread exceptions into test
errors. This surfaced as flaky failures on the pytest 7->9 bump (#228).

- polling_manager: wait on the stop event instead of time.sleep so stop()
  interrupts the interval immediately and the thread joins promptly, and
  guard update_environment so an unexpected error never kills the thread.
- streaming_manager: catch any exception in the run loop rather than a
  fixed tuple, so the thread never propagates an unhandled exception.
- tests: join (not just stop) the threads on teardown so none outlive
  their test, and keep the realtime test's stream worker off the network.

beep boop
@khvn26 khvn26 requested a review from a team as a code owner June 26, 2026 17:59
@khvn26 khvn26 requested review from emyller and removed request for a team June 26, 2026 17:59

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves thread management and error handling in the polling and streaming managers. Specifically, it replaces time.sleep with _stop_event.wait in the polling manager for immediate interruption, catches generic exceptions in both managers to prevent threads from dying, and joins background threads during test teardown to avoid race conditions with pyfakefs. The review feedback suggests adding a wait interval in the streaming manager's exception handler to prevent a high-CPU busy loop, and joining the event processor thread during test teardown to ensure proper cleanup.

Comment thread flagsmith/streaming_manager.py
Comment thread tests/conftest.py
@khvn26 khvn26 merged commit 0615a9d into main Jun 26, 2026
6 checks passed
@khvn26 khvn26 deleted the fix/leaked-threads-pytest9 branch June 26, 2026 18:54
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.

3 participants