From 5de287b9637b421e943defc16ecbeb32fe2e5650 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Fri, 31 Jul 2026 06:53:57 +0000 Subject: [PATCH] [Python] Fix crash unpickling a ROOT object without importing cppyy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unpickling a ROOT object resolves its reduce callable `ROOT.libROOTPythonizations._CPPInstance__expand__`, which only imports the ROOT pythonizations extension and not the cppyy backend. It then enters CPyCppyy through the public API (Instance_FromVoidPtr) to rebuild the object. In that state, `CPyCppyy::Initialize()` only imported the cppyy module in the branch taken when Python was not yet initialized. When Python is already running (the normal unpickling case), that branch is skipped and `Initialize()` returned success without ever importing cppyy, leaving gThisModule null. `CreateScopeProxy()` then dereferenced it as a fake scope and crashed with a segmentation violation. Ensure the cppyy extension module is imported whenever `gThisModule` is still null, so the public API is usable regardless of how CPyCppyy was entered. Add a regression test that pickles a ROOT object and reads it back in a fresh interpreter where cppyy has not been imported yet. 🤖 Done with the help of AI. --- bindings/pyroot/cppyy/CPyCppyy/src/API.cxx | 12 +++- .../pyroot/pythonizations/test/CMakeLists.txt | 2 + .../test/regression_pickle_no_cppyy.py | 64 +++++++++++++++++++ 3 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 bindings/pyroot/pythonizations/test/regression_pickle_no_cppyy.py diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/API.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/API.cxx index 48cb1f5377ed9..63e698163524b 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/src/API.cxx +++ b/bindings/pyroot/cppyy/CPyCppyy/src/API.cxx @@ -79,11 +79,17 @@ static bool Initialize() #if PY_VERSION_HEX < 0x03080000 PySys_SetArgv(sizeof(argv)/sizeof(argv[0]), argv); #endif - - // force loading of the cppyy module - PyRun_SimpleString(const_cast("import cppyy")); } +// Make sure the cppyy extension module is imported, as this is what runs the +// CPyCppyy module initialization that sets up internals such as gThisModule. + if (!CPyCppyy::gThisModule) { + PyObject* cppyymod = PyImport_ImportModule("cppyy"); + if (!cppyymod) + return false; + Py_DECREF(cppyymod); + } + if (!gMainDict) { // retrieve the main dictionary gMainDict = PyModule_GetDict( diff --git a/bindings/pyroot/pythonizations/test/CMakeLists.txt b/bindings/pyroot/pythonizations/test/CMakeLists.txt index b7cdc2e3bf5fa..6d705259f5af8 100644 --- a/bindings/pyroot/pythonizations/test/CMakeLists.txt +++ b/bindings/pyroot/pythonizations/test/CMakeLists.txt @@ -200,6 +200,8 @@ endif() ROOT_ADD_PYUNITTEST(regression_18441 regression_18441.py) +ROOT_ADD_PYUNITTEST(regression_pickle_no_cppyy regression_pickle_no_cppyy.py) + ROOT_ADD_PYUNITTEST(package_metadata package_metadata.py) if(NOT MSVC OR CMAKE_SIZEOF_VOID_P EQUAL 8 OR win_broken_tests) diff --git a/bindings/pyroot/pythonizations/test/regression_pickle_no_cppyy.py b/bindings/pyroot/pythonizations/test/regression_pickle_no_cppyy.py new file mode 100644 index 0000000000000..1683ba3ee517e --- /dev/null +++ b/bindings/pyroot/pythonizations/test/regression_pickle_no_cppyy.py @@ -0,0 +1,64 @@ +import os +import pickle +import subprocess +import sys +import tempfile +import textwrap +import unittest + + +class RegressionPickleNoCppyy(unittest.TestCase): + """ + Reading back a pickled ROOT object in a fresh interpreter, where the cppyy + backend has not been imported yet, must not crash. + + Unpickling a ROOT object only imports ``ROOT.libROOTPythonizations`` (to + resolve ``_CPPInstance__expand__``), which does not initialize the cppyy + backend. Entering CPyCppyy through the public API in that state used to + leave ``gThisModule`` null, so that ``CreateScopeProxy`` crashed with a + segmentation violation when using it as a fake scope. + + See the ROOT forum report: + https://root-forum.cern.ch/t/issue-with-new-root-version-on-lxplus + """ + + def test_load_without_prior_cppyy_import(self): + import ROOT + + # Create a pickle of a ROOT object. Touching a class here imports the + # cppyy backend, but only in this (throw-away) process. + h = ROOT.TH1F("h", "h", 10, 0, 1) + h.Fill(0.5) + + fname = tempfile.NamedTemporaryFile(suffix=".pkl", delete=False).name + try: + with open(fname, "wb") as f: + pickle.dump(h, f) + + # Read it back in a *fresh* interpreter that only imports pickle, so + # that the cppyy backend is not initialized before the object is + # expanded. This reproduces the exact scenario from the bug report. + code = textwrap.dedent( + """ + import pickle + with open({fname!r}, "rb") as f: + h = pickle.load(f) + assert h.GetName() == "h", h.GetName() + assert h.GetEntries() == 1.0, h.GetEntries() + """ + ).format(fname=fname) + + # Do not use check=True so we can give a helpful message on crash + # (a segfault shows up as a negative return code on POSIX). + proc = subprocess.run([sys.executable, "-c", code]) + self.assertEqual( + proc.returncode, + 0, + "Unpickling a ROOT object in a fresh interpreter crashed (return code {}).".format(proc.returncode), + ) + finally: + os.remove(fname) + + +if __name__ == "__main__": + unittest.main()