fix(gee): close the EESession HTTP client on GEEInterface.close() - #1000
Merged
Conversation
close() stopped the loop and thread but never awaited session.aclose(), leaving the httpx AsyncClient (HTTP/2 pool, sockets, TLS state) to the garbage collector. Run aclose() on the session's own loop, with a 5 s timeout, before stopping it.
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
1 participant
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.
Problem
GEEInterface.close()stops the event loop and joins its thread, but never awaitssession.aclose(). TheEESession'shttpx.AsyncClient(HTTP/2 pool, sockets, TLS state) is abandoned to the garbage collector, so its release on kernel cull is non-deterministic — connections linger until whenever GC runs, and any accidental surviving reference would turn it into a real leak.(Found during the se.plan memory investigation. Note this is NOT the RSS-ratchet fix — that one is allocator-level, sepal-contrib/se.plan#275 — this is deterministic-teardown hygiene, measured as not leaking today thanks to GC.)
Fix
Await
session.aclose()on the session's own loop (run_coroutine_threadsafe, 5 s timeout) before stopping the loop — ordering matters, aclose needs the loop alive. Failures are logged and never block shutdown;close()stays idempotent.Tests
Three unit tests in
tests/test_scripts/test_gee_interface_close.py, no GEE credentials needed (stub session): aclose runs exactly once on the interface's loop thread; no-session close still clean; an explodingaclose()doesn't block shutdown and double-close is a no-op. Verified red before the fix, green after; rest of the non-GEE suite unchanged (345 passed; the pre-existing network/credential failures are identical on unmodified main).