[Python] Delete TBrowser instances at exit#22200
Open
guitargeek wants to merge 1 commit into
Open
Conversation
Closes root-project#21912, which reported that a Python session that constructs a TRootBrowser segfaults at exit inside `TROOT::EndOfProcessCleanups()`. To fix this, register an atexit hook that calls `gROOT->GetListOfBrowsers()->Delete()` while the Python interpreter is still alive. This is the same workaround suggested in the issue, just made automatic. Honors `PyConfig.ShutDown`, and short-circuits if `_finalSetup()` never ran (no ROOT C++ runtime means nothing to clean up, and we do not want to drag cppyy in). Why we cannot just call `EndOfProcessCleanups()` from atexit? That was the previous behaviour, removed in commit e9d2803 ("[python] Avoid interfering with gc at exit time"). Doing this would re-introduce test failures that the removal commit as fixed: ``` pyunittests-bindings-pyroot-pythonizations-pyroot-pyz-tdirectory-attrsyntax pyunittests-bindings-pyroot-pythonizations-pyroot-pyz-tdirectoryfile-attrsyntax-get ``` `EndOfProcessCleanups()` tears down C++ objects that the TestCase classes still hold via CPyCppyy proxies (TFile / TDirectory). When CPython's gc walks those classes, it deallocates the proxies and reads memory ROOT just freed. Limiting the exit-time work to `TROOT::fBrowsers` leaves such objects untouched, so those tests stay green.
1 task
Test Results 22 files 22 suites 3d 17h 2m 25s ⏱️ For more details on these failures, see this check. Results for commit ece41ea. ♻️ This comment has been updated with latest results. |
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.
Closes #21912, which reported that a Python session that constructs a TRootBrowser segfaults at exit inside
TROOT::EndOfProcessCleanups().To fix this, register an atexit hook that calls
gROOT->GetListOfBrowsers()->Delete()while the Python interpreter is still alive. This is the same workaround suggested in the issue, just made automatic. HonorsPyConfig.ShutDown, and short-circuits if_finalSetup()never ran (no ROOT C++ runtime means nothing to clean up, and we do not want to drag cppyy in).Why we cannot just call
EndOfProcessCleanups()from atexit? That was the previous behaviour, removed in commit e9d2803 ("[python] Avoid interfering with gc at exit time"). Doing this would re-introduce test failures that the removal commit as fixed:EndOfProcessCleanups()tears down C++ objects that the TestCase classes still hold via CPyCppyy proxies (TFile / TDirectory). When CPython's gc walks those classes, it deallocates the proxies and reads memory ROOT just freed. Limiting the exit-time work toTROOT::fBrowsersleaves such objects untouched, so those tests stay green.