[ROOT-10469] Introduce two teardown modes for PyROOT: soft and hard

etejedor · etejedor · commit 7e47c421a88f · 2019-12-13T13:15:48.000+01:00
Motivation: we need to make sure that, if PyROOT is used from
another process that will keep on living after the Python
interpreter dies, PyROOT does not shut down the ROOT interpreter
via TROOT::EndOfProcessCleanups when running its atexit handler.

In that sense, this commit adds a configuration option to tell
PyROOT if the teardown needs to be soft, i.e. we do not want to
shut down the ROOT interpreter. Instead, in the soft mode, we
we only want (and need) to clean the objects that are controlled
by PyROOT via its TMemoryRegulator.
diff --git a/bindings/pyroot/ROOT.py b/bindings/pyroot/ROOT.py
@@ -168,14 +168,15 @@ def ismethod( object ):
 ### configuration ---------------------------------------------------------------
 class _Configuration( object ):
    __slots__ = [ 'IgnoreCommandLineOptions', 'StartGuiThread', 'ExposeCppMacros',
-                 '_gts', 'DisableRootLogon' ]
+                 '_gts', 'DisableRootLogon', 'ShutDown' ]
 
    def __init__( self ):
       self.IgnoreCommandLineOptions = 0
       self.StartGuiThread = True
       self.ExposeCppMacros = False
       self._gts = []
       self.DisableRootLogon = False
+      self.ShutDown = True
 
    def __setGTS( self, value ):
       for c in value:
@@ -866,21 +867,29 @@ def cleanup():
    del v, k, items, types
 
  # destroy facade
+   shutdown = PyConfig.ShutDown
    facade.__dict__.clear()
    del facade
 
    if 'libPyROOT' in sys.modules:
-    # run part the gROOT shutdown sequence ... running it here ensures that
-    # it is done before any ROOT libraries are off-loaded, with unspecified
-    # order of static object destruction;
-      gROOT = sys.modules[ 'libPyROOT' ].gROOT
-      gROOT.EndOfProcessCleanups()
-      del gROOT
+      pyroot_backend = sys.modules['libPyROOT']
+      if shutdown:
+       # run part the gROOT shutdown sequence ... running it here ensures that
+       # it is done before any ROOT libraries are off-loaded, with unspecified
+       # order of static object destruction;
+         pyroot_backend.ClearProxiedObjects()
+         pyroot_backend.gROOT.EndOfProcessCleanups()
+      else:
+       # Soft teardown
+       # Make sure all the objects regulated by PyROOT are deleted and their
+       # Python proxies are properly nonified.
+         pyroot_backend.ClearProxiedObjects()
 
     # cleanup cached python strings
-      sys.modules[ 'libPyROOT' ]._DestroyPyStrings()
+      pyroot_backend._DestroyPyStrings()
 
     # destroy ROOT extension module
+      del pyroot_backend
       del sys.modules[ 'libPyROOT' ]
 
  # destroy ROOT module
diff --git a/bindings/pyroot/src/RootModule.cxx b/bindings/pyroot/src/RootModule.cxx
@@ -798,6 +798,8 @@ static PyMethodDef gPyROOTMethods[] = {
      METH_NOARGS, (char*) "PyROOT internal function" },
    { (char*) "_ResetRootModule", (PyCFunction)RootModuleResetCallback,
      METH_NOARGS, (char*) "PyROOT internal function" },
+   { (char*) "ClearProxiedObjects", (PyCFunction)ClearProxiedObjects,
+     METH_NOARGS, (char*) "PyROOT internal function" },
    { (char*) "AddressOf", (PyCFunction)AddressOf,
      METH_VARARGS, (char*) "Retrieve address of held object in a buffer" },
    { (char*) "addressof", (PyCFunction)addressof,
diff --git a/bindings/pyroot/src/RootWrapper.cxx b/bindings/pyroot/src/RootWrapper.cxx
@@ -175,15 +175,19 @@ namespace {
 } // unnamed namespace
 
 
+static TMemoryRegulator &GetMemoryRegulator() {
+   static TMemoryRegulator m;
+   return m;
+}
+
 //- public functions ---------------------------------------------------------
 void PyROOT::InitRoot()
 {
 // setup interpreter locks to allow for threading in ROOT
    PyEval_InitThreads();
 
 // memory management
-   static TMemoryRegulator m;
-   gROOT->GetListOfCleanups()->Add( &m );
+   gROOT->GetListOfCleanups()->Add( &GetMemoryRegulator() );
 
 // bind ROOT globals that are needed in ROOT.py
    AddToGlobalScope( "gROOT", "TROOT.h", gROOT, Cppyy::GetScope( gROOT->IsA()->GetName() ) );
@@ -810,6 +814,15 @@ PyObject* PyROOT::GetCppGlobal( const std::string& name )
    return 0;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+/// Delete all memory-regulated objects
+
+PyObject *PyROOT::ClearProxiedObjects()
+{
+   GetMemoryRegulator().ClearProxiedObjects();
+   Py_RETURN_NONE;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 /// only known or knowable objects will be bound (null object is ok)
 
diff --git a/bindings/pyroot/src/RootWrapper.h b/bindings/pyroot/src/RootWrapper.h
@@ -27,6 +27,9 @@ namespace PyROOT {
    PyObject* GetCppGlobal( const std::string& name );
    PyObject* GetCppGlobal( PyObject*, PyObject* args );
 
+// clean up all objects controlled by TMemoryRegulator
+   PyObject *ClearProxiedObjects();
+
 // bind a ROOT object into a Python object
    PyObject* BindCppObjectNoCast( Cppyy::TCppObject_t object, Cppyy::TCppType_t klass,
       Bool_t isRef = kFALSE, Bool_t isValue = kFALSE );
diff --git a/bindings/pyroot/src/TMemoryRegulator.cxx b/bindings/pyroot/src/TMemoryRegulator.cxx
@@ -191,6 +191,29 @@ void PyROOT::TMemoryRegulator::RecursiveRemove( TObject* object )
    }
 }
 
+////////////////////////////////////////////////////////////////////////////////
+/// clean up all tracked objects
+
+void PyROOT::TMemoryRegulator::ClearProxiedObjects()
+{
+   while (!fgObjectTable->empty()) {
+      auto elem = fgObjectTable->begin();
+      auto cppobj = elem->first;
+      auto pyobj = (ObjectProxy*)PyWeakref_GetObject(elem->second);
+
+      if (pyobj && (pyobj->fFlags & ObjectProxy::kIsOwner)) {
+         // Only delete the C++ object if the Python proxy owns it.
+         // The deletion will trigger RecursiveRemove on the object
+         delete cppobj;
+      }
+      else {
+         // Non-owning proxy, just unregister to clean tables.
+         // The proxy deletion by Python will have no effect on C++, so all good
+         UnregisterObject(cppobj);
+      }
+   }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 /// start tracking <object> proxied by <pyobj>
 
diff --git a/bindings/pyroot/src/TMemoryRegulator.h b/bindings/pyroot/src/TMemoryRegulator.h
@@ -8,7 +8,7 @@
 #include "TObject.h"
 
 // Standard
-#include <map>
+#include <unordered_map>
 
 
 namespace PyROOT {
@@ -29,6 +29,9 @@ namespace PyROOT {
    // callback for ROOT/CINT
       virtual void RecursiveRemove( TObject* object );
 
+   // cleanup of all tracked objects
+      void ClearProxiedObjects();
+
    // add a python object to the table of managed objects
       static Bool_t RegisterObject( ObjectProxy* pyobj, TObject* object );
 
@@ -42,8 +45,8 @@ namespace PyROOT {
       static PyObject* ObjectEraseCallback( PyObject*, PyObject* pyref );
 
    private:
-      typedef std::map< TObject*, PyObject* > ObjectMap_t;
-      typedef std::map< PyObject*, ObjectMap_t::iterator > WeakRefMap_t;
+      typedef std::unordered_map< TObject*, PyObject* > ObjectMap_t;
+      typedef std::unordered_map< PyObject*, ObjectMap_t::iterator > WeakRefMap_t;
 
       static ObjectMap_t*  fgObjectTable;
       static WeakRefMap_t* fgWeakRefTable;
