Cppyy interop migration (draft PR for CI)

bindings/distrdf/python/DistRDF/Backends/Base.py

@@ -187,6 +187,19 @@ class BaseBackend(ABC):
     shared_libraries = set()
     strings_to_declare = dict()
 
+    def __init__(self):
+        # Enable ROOT thread safety on the client side. Distributed backends
+        # drive ROOT from more than one thread in the user's process: besides
+        # the main thread, the scheduler client (e.g. the Dask IO loop thread)
+        # deserializes the ROOT result objects coming back from the workers.
+        # That deserialization goes through the cling interpreter (TClass,
+        # wrapper compilation, ...), so it can run concurrently with interpreter
+        # activity on the main thread. Without thread safety enabled the
+        # unsynchronized access corrupts the interpreter state and leads to
+        # sporadic crashes. This mirrors the ROOT.EnableThreadSafety() call done
+        # on the workers in setup_mapper().
+        ROOT.EnableThreadSafety()
+
     @classmethod
     def register_initialization(cls, fun, *args, **kwargs):
         """

bindings/pyroot/cppyy/CPyCppyy/include/CPyCppyy/API.h

@@ -25,19 +25,27 @@
 #endif
 #include "Python.h"
 
-#define CPYCPPYY_VERSION_HEX 0x010c10
+#define CPYCPPYY_VERSION_HEX 0x011200
 
 // Cppyy types
+#ifndef CPYCPPYY_INTERNAL
+
+namespace Cpp {
+struct DeclRef;
+struct TypeRef;
+struct FuncRef;
+struct ObjectRef;
+}  // namespace Cpp
+
 namespace Cppyy {
-    typedef size_t      TCppScope_t;
-    typedef TCppScope_t TCppType_t;
-    typedef void*       TCppEnum_t;
-    typedef void*       TCppObject_t;
-    typedef intptr_t    TCppMethod_t;
-
-    typedef size_t      TCppIndex_t;
-    typedef void*       TCppFuncAddr_t;
+typedef Cpp::DeclRef TCppScope_t;
+typedef Cpp::TypeRef TCppType_t;
+typedef Cpp::ObjectRef TCppObject_t;
+typedef Cpp::FuncRef TCppMethod_t;
+typedef size_t TCppIndex_t;
+typedef void* TCppFuncAddr_t;
 } // namespace Cppyy
+#endif
 
 // Bindings
 #include "CPyCppyy/CommonDefs.h"
@@ -123,6 +131,7 @@ class CPYCPPYY_CLASS_EXTERN Converter {
 
 // create a converter based on its full type name and dimensions
 CPYCPPYY_EXTERN Converter* CreateConverter(const std::string& name, cdims_t = 0);
+CPYCPPYY_EXTERN Converter* CreateConverter(Cppyy::TCppType_t type, cdims_t = 0);
 
 // delete a previously created converter
 CPYCPPYY_EXTERN void DestroyConverter(Converter* p);
@@ -153,6 +162,7 @@ class CPYCPPYY_CLASS_EXTERN Executor {
 
 // create an executor based on its full type name
 CPYCPPYY_EXTERN Executor* CreateExecutor(const std::string& name, cdims_t = 0);
+CPYCPPYY_EXTERN Executor* CreateExecutor(Cppyy::TCppType_t type, cdims_t = 0);
 
 // delete a previously created executor
 CPYCPPYY_EXTERN void DestroyConverter(Converter* p);
@@ -183,7 +193,8 @@ CPYCPPYY_EXTERN void* Instance_AsVoidPtr(PyObject* pyobject);
 // void* to C++ Instance (python object proxy) conversion, returns a new reference
 CPYCPPYY_EXTERN PyObject* Instance_FromVoidPtr(
     void* addr, const std::string& classname, bool python_owns = false);
-
+CPYCPPYY_EXTERN PyObject* Instance_FromVoidPtr(
+    void* addr, Cppyy::TCppScope_t klass_scope, bool python_owns = false);
 // type verifiers for C++ Scope
 CPYCPPYY_EXTERN bool Scope_Check(PyObject* pyobject);
 CPYCPPYY_EXTERN bool Scope_CheckExact(PyObject* pyobject);

bindings/pyroot/cppyy/CPyCppyy/include/CPyCppyy/CommonDefs.h

@@ -6,29 +6,29 @@
 #ifdef _MSC_VER
 // Windows requires symbols to be explicitly exported
 #define CPYCPPYY_EXPORT extern __declspec(dllexport)
+#define CPYCPPYY_IMPORT extern __declspec(dllimport)
 #define CPYCPPYY_CLASS_EXPORT __declspec(dllexport)
 
 // CPYCPPYY_EXTERN is dual use in the public API
 #ifndef CPYCPPYY_INTERNAL
 #define CPYCPPYY_EXTERN extern __declspec(dllexport)
 #define CPYCPPYY_CLASS_EXTERN __declspec(dllexport)
 #else
-#define CPYCPPYY_EXTERN extern
-#define CPYCPPYY_CLASS_EXTERN
+#define CPYCPPYY_EXTERN extern __declspec(dllimport)
+#define CPYCPPYY_CLASS_EXTERN __declspec(dllimport)
 #endif
 
 #define CPYCPPYY_STATIC
 
 #else
 // Linux, Mac, etc.
 #define CPYCPPYY_EXPORT extern
+#define CPYCPPYY_IMPORT extern
 #define CPYCPPYY_CLASS_EXPORT
 #define CPYCPPYY_EXTERN extern
 #define CPYCPPYY_CLASS_EXTERN
 #define CPYCPPYY_STATIC static
 
 #endif
 
-#define CPYCPPYY_IMPORT extern
-
 #endif // !CPYCPPYY_COMMONDEFS_H

bindings/pyroot/cppyy/CPyCppyy/include/CPyCppyy/DispatchPtr.h

@@ -16,9 +16,16 @@
 
 // Bindings
 #include "CPyCppyy/CommonDefs.h"
-
+#include <Python.h>
 
 namespace CPyCppyy {
+class PythonGILRAII {
+  PyGILState_STATE state;
+
+public:
+  PythonGILRAII() : state(PyGILState_Ensure()) {}
+  ~PythonGILRAII() { PyGILState_Release(state); }
+};
 
 class CPYCPPYY_CLASS_EXTERN DispatchPtr {
 public:

bindings/pyroot/cppyy/CPyCppyy/src/API.cxx

@@ -1,12 +1,14 @@
 // Bindings
 #include "CPyCppyy.h"
+#include "Cppyy.h"
 #define CPYCPPYY_INTERNAL 1
 #include "CPyCppyy/API.h"
 #undef CPYCPPYY_INTERNAL
 
 #include "CPPInstance.h"
 #include "CPPOverload.h"
 #include "CPPScope.h"
+#include "CPyCppyy/DispatchPtr.h"
 #include "ProxyWrappers.h"
 #include "PyStrings.h"
 
@@ -46,22 +48,10 @@ static bool Initialize()
 
     if (!Py_IsInitialized()) {
     // this happens if Cling comes in first
-#if PY_VERSION_HEX < 0x03020000
-        PyEval_InitThreads();
-#endif
-#if PY_VERSION_HEX < 0x03080000
-        Py_Initialize();
-#else
         PyConfig config;
         PyConfig_InitPythonConfig(&config);
         PyConfig_SetString(&config, &config.program_name, L"cppyy");
         Py_InitializeFromConfig(&config);
-#endif
-#if PY_VERSION_HEX >= 0x03020000
-#if PY_VERSION_HEX < 0x03090000
-        PyEval_InitThreads();
-#endif
-#endif
 
     // try again to see if the interpreter is initialized
         if (!Py_IsInitialized()) {
@@ -70,21 +60,12 @@ static bool Initialize()
             return false;
         }
 
-   // set the command line arguments on python's sys.argv
-#if PY_VERSION_HEX < 0x03000000
-        char* argv[] = {const_cast<char*>("cppyy")};
-#elif PY_VERSION_HEX < 0x03080000
-        wchar_t* argv[] = {const_cast<wchar_t*>(L"cppyy")};
-#endif
-#if PY_VERSION_HEX < 0x03080000
-        PySys_SetArgv(sizeof(argv)/sizeof(argv[0]), argv);
-#endif
-
     // force loading of the cppyy module
         PyRun_SimpleString(const_cast<char*>("import cppyy"));
     }
 
     if (!gMainDict) {
+        CPyCppyy::PythonGILRAII python_gil_raii;
     // retrieve the main dictionary
         gMainDict = PyModule_GetDict(
             PyImport_AddModule(const_cast<char*>("__main__")));
@@ -110,7 +91,7 @@ std::string CPyCppyy::Instance_GetScopedFinalName(PyObject* pyobject)
        return "";
    }
 
-   Cppyy::TCppType_t pyobjectClass = ((CPPInstance *)pyobject)->ObjectIsA();
+   Cppyy::TCppScope_t pyobjectClass = ((CPPInstance *)pyobject)->ObjectIsA();
    return Cppyy::GetScopedFinalName(pyobjectClass);
 }
 
@@ -121,6 +102,8 @@ void* CPyCppyy::Instance_AsVoidPtr(PyObject* pyobject)
     if (!Initialize())
         return nullptr;
 
+    PythonGILRAII python_gil_raii;
+
 // check validity of cast
     if (!CPPInstance_Check(pyobject))
         return nullptr;
@@ -137,6 +120,8 @@ PyObject* CPyCppyy::Instance_FromVoidPtr(
     if (!Initialize())
         return nullptr;
 
+    PythonGILRAII python_gil_raii;
+
 // perform cast (the call will check TClass and addr, and set python errors)
     PyObject* pyobject = BindCppObjectNoCast(addr, Cppyy::GetScope(classname), false);
 
@@ -147,6 +132,25 @@ PyObject* CPyCppyy::Instance_FromVoidPtr(
     return pyobject;
 }
 
+//-----------------------------------------------------------------------------
+PyObject* CPyCppyy::Instance_FromVoidPtr(
+    void* addr, Cppyy::TCppScope_t klass_scope, bool python_owns)
+{
+// Bind the addr to a python object of class defined by classname.
+    if (!Initialize())
+        return nullptr;
+
+    PythonGILRAII python_gil_raii;
+
+// perform cast (the call will check TClass and addr, and set python errors)
+    PyObject* pyobject = BindCppObjectNoCast(addr, klass_scope, false);
+
+// give ownership, for ref-counting, to the python side, if so requested
+    if (python_owns && CPPInstance_Check(pyobject))
+        ((CPPInstance*)pyobject)->PythonOwns();
+
+    return pyobject;
+}
 namespace CPyCppyy {
 // version with C type arguments only for use with Numba
 PyObject* Instance_FromVoidPtr(void* addr, const char* classname, int python_owns) {
@@ -161,6 +165,7 @@ bool CPyCppyy::Scope_Check(PyObject* pyobject)
     if (!Initialize())
         return false;
 
+    PythonGILRAII python_gil_raii;
     return CPPScope_Check(pyobject);
 }
 
@@ -171,6 +176,7 @@ bool CPyCppyy::Scope_CheckExact(PyObject* pyobject)
     if (!Initialize())
         return false;
 
+    PythonGILRAII python_gil_raii;
     return CPPScope_CheckExact(pyobject);
 }
 
@@ -181,6 +187,7 @@ bool CPyCppyy::Instance_Check(PyObject* pyobject)
     if (!Initialize())
         return false;
 
+    PythonGILRAII python_gil_raii;    
 // detailed walk through inheritance hierarchy
     return CPPInstance_Check(pyobject);
 }
@@ -192,6 +199,7 @@ bool CPyCppyy::Instance_CheckExact(PyObject* pyobject)
     if (!Initialize())
         return false;
 
+    PythonGILRAII python_gil_raii;
 // direct pointer comparison of type member
     return CPPInstance_CheckExact(pyobject);
 }
@@ -225,6 +233,7 @@ void CPyCppyy::Instance_SetCppOwns(PyObject* pyobject)
 //-----------------------------------------------------------------------------
 bool CPyCppyy::Sequence_Check(PyObject* pyobject)
 {
+    PythonGILRAII python_gil_raii;
 // Extends on PySequence_Check() to determine whether an object can be iterated
 // over (technically, all objects can b/c of C++ pointer arithmetic, hence this
 // check isn't 100% accurate, but neither is PySequence_Check()).
@@ -258,6 +267,7 @@ bool CPyCppyy::Sequence_Check(PyObject* pyobject)
 //-----------------------------------------------------------------------------
 bool CPyCppyy::Instance_IsLively(PyObject* pyobject)
 {
+    PythonGILRAII python_gil_raii;
 // Test whether the given instance can safely return to C++
     if (!CPPInstance_Check(pyobject))
         return true;    // simply don't know
@@ -277,6 +287,7 @@ bool CPyCppyy::Overload_Check(PyObject* pyobject)
     if (!Initialize())
         return false;
 
+    PythonGILRAII python_gil_raii;
 // detailed walk through inheritance hierarchy
     return CPPOverload_Check(pyobject);
 }
@@ -288,6 +299,7 @@ bool CPyCppyy::Overload_CheckExact(PyObject* pyobject)
     if (!Initialize())
         return false;
 
+    PythonGILRAII python_gil_raii;
 // direct pointer comparison of type member
     return CPPOverload_CheckExact(pyobject);
 }
@@ -305,6 +317,8 @@ bool CPyCppyy::Import(const std::string& mod_name)
     if (!Initialize())
         return false;
 
+    PythonGILRAII python_gil_raii;
+
     PyObject* mod = PyImport_ImportModule(mod_name.c_str());
     if (!mod) {
         PyErr_Print();
@@ -364,6 +378,8 @@ void CPyCppyy::ExecScript(const std::string& name, const std::vector<std::string
     if (!Initialize())
         return;
 
+    PythonGILRAII python_gil_raii;
+
 // verify arguments
     if (name.empty()) {
         std::cerr << "Error: no file name specified." << std::endl;
@@ -388,7 +404,6 @@ void CPyCppyy::ExecScript(const std::string& name, const std::vector<std::string
 // build new argv
     const int argc = (int)args.size() + 1;
     std::vector<wchar_t*> wargv(argc);
-
     wargv[0] = Py_DecodeLocale(name.c_str(), nullptr);
 
     for (int i = 1; i < argc; ++i) {
@@ -438,6 +453,7 @@ bool CPyCppyy::Exec(const std::string& cmd)
     if (!Initialize())
         return false;
 
+    PythonGILRAII python_gil_raii;
 // execute the command
     PyObject* result =
         PyRun_String(const_cast<char*>(cmd.c_str()), Py_file_input, gMainDict, gMainDict);
@@ -459,6 +475,7 @@ void CPyCppyy::Prompt() {
     if (!Initialize())
         return;
 
+    PythonGILRAII python_gil_raii;
 // enter i/o interactive mode
     PyRun_InteractiveLoop(stdin, const_cast<char*>("\0"));
 }

bindings/pyroot/cppyy/CPyCppyy/src/CPPClassMethod.cxx

@@ -5,11 +5,8 @@
 
 
 //- public members --------------------------------------------------------------
-PyObject* CPyCppyy::CPPClassMethod::Call(CPPInstance*&
-#if PY_VERSION_HEX >= 0x03080000
-    self
-#endif
-    , CPyCppyy_PyArgs_t args, size_t nargsf, PyObject* kwds, CallContext* ctxt)
+PyObject *CPyCppyy::CPPClassMethod::Call(CPPInstance *&self, CPyCppyy_PyArgs_t args,
+                                         size_t nargsf, PyObject *kwds, CallContext *ctxt)
 {
 // preliminary check in case keywords are accidently used (they are ignored otherwise)
     if (kwds && ((PyDict_Check(kwds) && PyDict_Size(kwds)) ||
@@ -23,7 +20,6 @@ PyObject* CPyCppyy::CPPClassMethod::Call(CPPInstance*&
         return nullptr;
 
 // translate the arguments
-#if PY_VERSION_HEX >= 0x03080000
 // TODO: The following is not robust and should be revisited e.g. by making CPPOverloads
 // that have only CPPClassMethods be true Python classmethods? Note that the original
 // implementation wasn't 100% correct either (e.g. static size() mapped to len()).
@@ -35,12 +31,11 @@ PyObject* CPyCppyy::CPPClassMethod::Call(CPPInstance*&
     if ((!self || (PyObject*)self == Py_None) && nargs) {
         PyObject* arg0 = CPyCppyy_PyArgs_GET_ITEM(args, 0);
         if (CPPInstance_Check(arg0) && fArgsRequired <= nargs - 1 &&
-            Cppyy::IsSubtype(reinterpret_cast<CPPInstance *>(arg0)->ObjectIsA(), GetScope())) {
+            Cppyy::IsSubclass(reinterpret_cast<CPPInstance *>(arg0)->ObjectIsA(), GetScope())) {
             args   += 1;     // drops first argument
             nargsf -= 1;
         }
     }
-#endif
 
     if (!this->ConvertAndSetArgs(args, nargsf, ctxt))
         return nullptr;