From f2ba388a80b9911bb846baacdaa31fd0be4ad014 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Tue, 27 Oct 2026 09:25:33 +0000 Subject: [PATCH 1/2] [Python] Add regression test for the exit-time crash of issue #23728 Reads a tree with a branch of a dictionary-less collection in a fresh Python process, lets TROOT::EndOfProcessCleanups delete it at exit, and constructs another dictionary-less object before exiting. This crashed in the JIT-compiled destructor lookup before the CppInterOp upgrade. The child process runs with MALLOC_PERTURB_ set, because a use of destroyed objects only crashes reliably when freed memory is poisoned, and with ROOT_OBJECT_AUTO_REGISTRATION=1 plus explicit checks that the file is registered with gROOT and owns the tree, so the test keeps exercising the scenario when object auto-registration is disabled. Assisted-by: claude-code:claude-fable-5-1 (cherry picked from commit a41307c1d22acbc53aeed4ef7cbfc4a37b6874f1) --- .../pyroot/pythonizations/test/CMakeLists.txt | 2 + .../pythonizations/test/regression_23728.py | 81 +++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 bindings/pyroot/pythonizations/test/regression_23728.py diff --git a/bindings/pyroot/pythonizations/test/CMakeLists.txt b/bindings/pyroot/pythonizations/test/CMakeLists.txt index 7738dfd103769..c793432addeeb 100644 --- a/bindings/pyroot/pythonizations/test/CMakeLists.txt +++ b/bindings/pyroot/pythonizations/test/CMakeLists.txt @@ -198,6 +198,8 @@ endif() ROOT_ADD_PYUNITTEST(regression_18441 regression_18441.py) +ROOT_ADD_PYUNITTEST(regression_23728 regression_23728.py) + if(NOT MSVC OR CMAKE_SIZEOF_VOID_P EQUAL 8 OR win_broken_tests) # Test the conversion and low level views of interpreter-defined C++ arrays to NumPy # We do not run this test on Windows x86 due to an interpreter issue with arrays: diff --git a/bindings/pyroot/pythonizations/test/regression_23728.py b/bindings/pyroot/pythonizations/test/regression_23728.py new file mode 100644 index 0000000000000..0371a664c691f --- /dev/null +++ b/bindings/pyroot/pythonizations/test/regression_23728.py @@ -0,0 +1,81 @@ +import os +import subprocess +import sys +import tempfile +import textwrap +import unittest + + +class Regression23728(unittest.TestCase): + """ + Deleting an object of a class without compiled dictionary during + TROOT::EndOfProcessCleanups() must not crash, even if the interpreter + already JIT-compiled a destructor earlier in the process. The cache of + compiled destructors used to be a function-local static, destroyed before + the end-of-process cleanup ran because its first use came after ROOT + registered its atexit handlers. + + See https://github.com/root-project/root/issues/23728 + """ + + def test_exit_cleanup_after_interpreted_destructor(self): + import ROOT + + with tempfile.TemporaryDirectory() as tmpdir: + fname = os.path.join(tmpdir, "vv.root") + + # vector> has no compiled dictionary, so its destructor + # has to be JIT-compiled when the branch object is deleted. + with ROOT.TFile(fname, "RECREATE") as f: + t = ROOT.TTree("t", "t") + v = ROOT.std.vector[ROOT.std.vector[int]]() + t.Branch("vv", v) + v.push_back(ROOT.std.vector[int]([1, 2])) + t.Fill() + f.Write() + + script = textwrap.dedent( + f""" + import ROOT + + f = ROOT.TFile({fname!r}) + # Let TROOT::EndOfProcessCleanups() delete the file, and with it + # the tree and its branch object, from the C atexit handler. + ROOT.SetOwnership(f, False) + t = f.Get("t") + t.GetEntry(0) + + # That only happens if the file is registered with gROOT and owns + # the tree. Check it, so that the test fails loudly instead of + # silently losing its purpose if these defaults change. + if not ROOT.gROOT.GetListOfFiles().FindObject(f): + raise RuntimeError("the file is not in gROOT->GetListOfFiles()") + if t.GetDirectory() != f: + raise RuntimeError("the tree is not owned by the file") + + # Make the interpreter JIT-compile a destructor before exit. + ROOT.std.vector[ROOT.TNamed]() + + print("end of script") + """ + ) + + # Poison freed memory, so that a use of destroyed objects (like the + # dead destructor cache of the issue) crashes deterministically, + # whatever the heap layout and the interpreter internals. + env = dict(os.environ, MALLOC_PERTURB_="85") + + res = subprocess.run( + [sys.executable, "-c", script], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + env=env, + ) + + self.assertEqual(res.returncode, 0, msg=res.stdout) + self.assertIn("end of script", res.stdout) + + +if __name__ == "__main__": + unittest.main() From 2c08af900f93024a49f468e161acf93e99d7ddb3 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Wed, 28 Oct 2026 19:42:22 +0000 Subject: [PATCH 2/2] [Python] Make the regression test for #23728 work effectively The test left the destructor of the branch class uncompiled until exit, so that cling had to JIT-compile it from TROOT::EndOfProcessCleanups(). On Windows, that cleanup runs from libCore's DLL_PROCESS_DETACH, after libCling's statics have already been torn down, so compiling anything there crashes with or without the fix (the no-fix run in #23850 fails identically). Pre-compile the exact destructor instead: the exit-time call is then a pure cache lookup, which is the regular exit path on Windows. The bug was in that lookup on the destroyed function-local static, so the test still fails without the fix. Write the branch through the overload taking the class name and the address. The templated Branch() that the pythonization on master calls refuses collections without a compiled CollectionProxy, which silently left the tree without the branch and the test without its purpose. Also give the child process a timeout: without the fix, ROOT's crash handler can hang instead of terminating the process. Assisted-by: claude-code:claude-fable-5-1 --- .../pythonizations/test/regression_23728.py | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/bindings/pyroot/pythonizations/test/regression_23728.py b/bindings/pyroot/pythonizations/test/regression_23728.py index 0371a664c691f..d3689530949c9 100644 --- a/bindings/pyroot/pythonizations/test/regression_23728.py +++ b/bindings/pyroot/pythonizations/test/regression_23728.py @@ -29,7 +29,11 @@ def test_exit_cleanup_after_interpreted_destructor(self): with ROOT.TFile(fname, "RECREATE") as f: t = ROOT.TTree("t", "t") v = ROOT.std.vector[ROOT.std.vector[int]]() - t.Branch("vv", v) + # Use the overload taking the class name and the address: the + # templated Branch() refuses collections without a compiled + # CollectionProxy, which would silently leave the tree without + # the branch and the test without its purpose. + t.Branch("vv", "vector >", ROOT.AddressOf(v)) v.push_back(ROOT.std.vector[int]([1, 2])) t.Fill() f.Write() @@ -53,8 +57,15 @@ def test_exit_cleanup_after_interpreted_destructor(self): if t.GetDirectory() != f: raise RuntimeError("the tree is not owned by the file") - # Make the interpreter JIT-compile a destructor before exit. - ROOT.std.vector[ROOT.TNamed]() + # Make the interpreter JIT-compile the destructor of the branch + # class before exit. This creates the destructor cache before the + # end-of-process cleanup needs it (and, before the fix, destroyed + # it again before that). Using the very same class keeps the + # exit-time destructor call a pure cache lookup: compiling code + # at that point is not possible on Windows, where the atexit + # handler of libCore runs only when libCling's statics have + # already been torn down by its DLL_PROCESS_DETACH. + ROOT.std.vector[ROOT.std.vector[int]]() print("end of script") """ @@ -65,12 +76,16 @@ def test_exit_cleanup_after_interpreted_destructor(self): # whatever the heap layout and the interpreter internals. env = dict(os.environ, MALLOC_PERTURB_="85") + # Without the fix, ROOT's crash handler can hang on the way out + # of the segfault instead of terminating the process: fail + # instead of waiting for the ctest timeout. res = subprocess.run( [sys.executable, "-c", script], stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, env=env, + timeout=300, ) self.assertEqual(res.returncode, 0, msg=res.stdout)