fix: restart local-eval polling thread after fork (#77)#203
Draft
fix: restart local-eval polling thread after fork (#77)#203
Conversation
The polling thread that refreshes the local evaluation environment document is created in the parent process. Threads do not survive os.fork(), so pre-fork servers (gunicorn, uwsgi, multiprocessing) end up with worker processes whose Python Thread object is intact but whose underlying OS thread is dead. Workers then serve a frozen environment-document snapshot from the moment of fork for their entire lifetime. EnvironmentDataPollingManager now composes a threading.Thread instead of inheriting from it, and registers an os.register_at_fork hook so a fresh polling thread is started in each child. The hook also closes the parent's requests.Session so connection-pool sockets are not shared across processes. Stream and analytics threads are not yet covered; tracking separately.
The pytest-httpserver dev dependency requires python>=3.10. Move the fork test (the only consumer) into its own file so the four pre-existing polling-manager tests still run on 3.9, and skip the new file there via importorskip. Add a mypy override so 3.9 type-checks do not error on the missing module.
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.
Closes #77.
The polling thread that refreshes the local evaluation environment document is created in the parent process. Threads do not survive
os.fork(), so pre-fork servers (gunicorn, uwsgi, multiprocessing) end up with worker processes whose PythonThreadobject is intact but whose underlying OS thread is dead.is_alive()correctly reportsFalse, no polls happen, and workers serve a frozen environment-document snapshot from the moment of fork for their entire lifetime.EnvironmentDataPollingManagernow composes athreading.Threadinstead of inheriting from it, and registers anos.register_at_fork(after_in_child=...)hook so a fresh polling thread is started in each child. The hook also closes the parent'srequests.Sessionbecause connection-pool sockets are inherited as shared FDs across fork; reusing them would interleave bytes between processes.Notes
Test plan
test_polling_manager_keeps_polling_after_forkspawns amultiprocessing.get_context("fork").Process, assertsis_alive=Truein the child, and asserts the child'sidentdiffers from the parent's (i.e. it is a fresh thread, not the dead inherited one).Notes