fix: resolve session lifecycle bugs in Solara session management#980
Merged
dfguerrerom merged 2 commits intomainfrom Mar 18, 2026
Merged
fix: resolve session lifecycle bugs in Solara session management#980dfguerrerom merged 2 commits intomainfrom
dfguerrerom merged 2 commits intomainfrom
Conversation
- Make create_session idempotent by skipping if kernel already has a session, preventing leaked GEEInterface threads on every Solara re-render - Fix GEEInterface.close() using wrong attribute names (_sync_loop/_sync_thread instead of _async_loop/_async_thread), which prevented cleanup from ever stopping the background event loop and thread - Raise RuntimeError in get_current_gee_interface/get_current_drive_interface when SessionManager is active but no session exists, instead of silently falling back to shared process-level credentials in multi-user deployments
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Solara session lifecycle handling in pysepal to prevent per-render session leaks, ensure GEEInterface background resources are actually cleaned up, and avoid silently falling back to shared process credentials when a per-kernel session is expected.
Changes:
- Make
SessionManager.create_session()idempotent per kernel to prevent recreating interfaces on re-renders. - Fix
GEEInterface.close()cleanup guards to reference the correct async loop/thread attributes. - Raise
RuntimeErrorin Solara contexts when no per-kernel session exists (instead of falling back to shared credentials).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pysepal/solara/utils.py |
Raises explicit errors when session manager is active but no kernel session exists; keeps fallback for non-Solara contexts. |
pysepal/solara/session_manager.py |
Adds an idempotency guard to avoid recreating sessions for the same kernel. |
pysepal/scripts/gee_interface.py |
Corrects close() to stop/close the async loop and join the correct background thread. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Convert kernel_id to str in get_kernel_id() so the return type matches the Dict[str, ...] type hints and method signatures - Extend test_close to verify _async_thread is stopped and _async_loop is closed after close(), preventing regression of the thread/loop leak fix
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.
Summary
create_sessionidempotent — skip creation if a session already exists for the kernel, preventing leakedGEEInterfacethreads/event loops on every Solara re-renderGEEInterface.close()— thehasattrguards checked_sync_loop/_sync_threadbut the constructor creates_async_loop/_async_thread, so cleanup never actually stopped the background loop or joined the threadget_current_gee_interface()andget_current_drive_interface()now raiseRuntimeErrorwhenSessionManageris active but no session exists for the current kernel, instead of silently falling back to shared process-level credentials. The fallback is preserved for non-Solara contexts (notebooks, scripts).Test plan
@with_sepal_sessionsand verify session is created once per kernel (check logs for "skipping creation" on re-renders)GEEInterface.close()stops the background thread on kernel shutdown (no lingering threads)get_current_gee_interface()without@with_sepal_sessionsin a Solara app raisesRuntimeErrorGEEInterfacewithout error