From ca11e291aab0a7808d8b5bbcd4efe6c4306b9e83 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Tue, 27 Oct 2026 07:48:24 +0000 Subject: [PATCH 1/3] [metacling] Move wrapper caches from local statics to file scope TClingCallFunc caches the JIT-compiled call, constructor and destructor wrappers in std::maps that were function-local statics. Such a static registers its destructor on first use, and for the destructor-wrapper cache the first use can be in the middle of the user's program, e.g. when PyROOT destroys an object of a class template instantiation without compiled dictionary. That is after ROOT registered its atexit handlers, so at exit the cache was destroyed before TROOT::EndOfProcessCleanups() deleted the files and trees still alive, and the destructor lookup for a branch object without compiled dictionary (e.g. std::vector>) ran on a dead map: segfault or "corrupted double-linked list" at exit, depending on the heap state. Making the maps globals of libCling ties their lifetime to the library, like the other interpreter globals. They are then constructed when libCling is loaded by TROOT::InitInterpreter(), i.e. before the atexit handlers that run the end-of-process cleanups are registered, and outlive them. This is the 6.36 version of the fix for #23728. In 6.40 and later the caches live in CppInterOp, which fixed the same issue by moving them to per-interpreter storage (CppInterOp commit b8ae9f666c). Assisted-by: claude-code:claude-fable-5-1 --- core/metacling/src/TClingCallFunc.cxx | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/core/metacling/src/TClingCallFunc.cxx b/core/metacling/src/TClingCallFunc.cxx index 7d4b93c8810e0..45583f9b78e09 100644 --- a/core/metacling/src/TClingCallFunc.cxx +++ b/core/metacling/src/TClingCallFunc.cxx @@ -79,6 +79,19 @@ using std::string, std::map, std::ostringstream, std::make_pair; static unsigned long long gWrapperSerial = 0LL; static const string kIndentString(" "); +// Caches of the JIT-compiled call, constructor and destructor wrappers, keyed on +// the declaration they were generated for. They are globals of libCling on +// purpose: a function-local static would register its destructor on first use, +// which for the destructor wrappers can be in the middle of the user's program, +// i.e. after ROOT registered its atexit handlers. The cache would then be +// destroyed before TROOT::EndOfProcessCleanups() releases the remaining objects +// (e.g. a TTree branch of a class without compiled dictionary), and the lookup +// of the destructor wrapper would run on a dead map, see +// https://github.com/root-project/root/issues/23728. +static map gWrapperStore; +static map gCtorWrapperStore; +static map gDtorWrapperStore; + static inline void @@ -1009,8 +1022,6 @@ void TClingCallFunc::make_narg_call_with_return(const unsigned N, const string & tcling_callfunc_Wrapper_t TClingCallFunc::make_wrapper() { - static map gWrapperStore; - R__LOCKGUARD_CLING(gInterpreterMutex); const Decl *D = GetFunctionOrShadowDecl(); @@ -1137,8 +1148,6 @@ tcling_callfunc_ctor_Wrapper_t TClingCallFunc::make_ctor_wrapper(const TClingCla // //-- - static map gCtorWrapperStore; - R__LOCKGUARD_CLING(gInterpreterMutex); auto D = info->GetDecl(); @@ -1287,8 +1296,6 @@ TClingCallFunc::make_dtor_wrapper(const TClingClassInfo *info) // //-- - static map gDtorWrapperStore; - R__LOCKGUARD_CLING(gInterpreterMutex); const Decl *D = info->GetDecl(); From dbc6fda64a4adb73ffde6139e55c8e78c038d097 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Tue, 27 Oct 2026 09:25:33 +0000 Subject: [PATCH 2/3] [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 9d2c54a844da5ccccb3c0d404881f9b2db3cf026 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Wed, 28 Oct 2026 19:42:22 +0000 Subject: [PATCH 3/3] [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 (cherry picked from commit 98526e5303004fc5d11c4ae8f281324d3b049179) --- .../pythonizations/test/regression_23728.py | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/bindings/pyroot/pythonizations/test/regression_23728.py b/bindings/pyroot/pythonizations/test/regression_23728.py index 0371a664c691f..243473c6b4f79 100644 --- a/bindings/pyroot/pythonizations/test/regression_23728.py +++ b/bindings/pyroot/pythonizations/test/regression_23728.py @@ -29,7 +29,12 @@ 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. + branch = t.Branch("vv", "vector >", ROOT.AddressOf(v)) + self.assertTrue(branch, "Branch() refused to create the branch") v.push_back(ROOT.std.vector[int]([1, 2])) t.Fill() f.Write() @@ -53,8 +58,22 @@ 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]() + # Sanity checks that the branch exists and that bytes are + # actually read, which is required for the reproducer. + if not t.GetBranch("vv"): + raise RuntimeError("the tree has no 'vv' branch") + if t.GetEntry(0) <= 0: + raise RuntimeError("no bytes read for entry 0: no branch object was created") + + # 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 +84,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)