fix: Background threads leaking across tests#229
Merged
Conversation
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
Contributor
There was a problem hiding this comment.
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.
matthewelwell
approved these changes
Jun 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
-W error).