From d956081fd3e761aaa62409335adc2f36a54ff09f Mon Sep 17 00:00:00 2001 From: Petr Mitrichev Date: Thu, 28 Jun 2018 14:27:52 +0200 Subject: [PATCH 1/4] Avoid function-local statics that call Python code to avoid deadlocks with GIL --- Lib/python/pyinit.swg | 5 ++++ Lib/python/pyrun.swg | 53 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/Lib/python/pyinit.swg b/Lib/python/pyinit.swg index 7ef70585f..ea865c066 100644 --- a/Lib/python/pyinit.swg +++ b/Lib/python/pyinit.swg @@ -431,6 +431,11 @@ SWIG_init(void) { SwigPyBuiltin_AddPublicSymbol(public_interface, swig_const_table[i].name); #endif + SWIG_Py_None_global_Init(); + SwigPyObject_type_global_Init(); + SwigPyPacked_type_global_Init(); + SWIG_Python_TypeCache_global_Init(); + SWIG_InstallConstants(d,swig_const_table); %} diff --git a/Lib/python/pyrun.swg b/Lib/python/pyrun.swg index 34b3a1f41..c0d96a014 100644 --- a/Lib/python/pyrun.swg +++ b/Lib/python/pyrun.swg @@ -258,11 +258,24 @@ _SWIG_Py_None(void) Py_DECREF(none); return none; } -SWIGRUNTIME PyObject * + +static PyObject *SWIG_Py_None_global = 0; + +SWIGRUNTIME void SWIG_Py_None_global_Init(void) { + SWIG_Py_None_global = _SWIG_Py_None(); +} + +SWIGRUNTIME PyObject * SWIG_Py_None(void) { - static PyObject *SWIG_STATIC_POINTER(none) = _SWIG_Py_None(); - return none; + assert(SWIG_Py_None_global); + return SWIG_Py_None_global; +} + +#else +SWIGRUNTIME void SWIG_Py_None_global_Init(void) +{ + // Do nothing } #endif @@ -488,11 +501,21 @@ SwigPyObject_type(void) { assert(cd->pytype); return cd->pytype; } + +SWIGRUNTIME void SwigPyObject_type_global_Init(void) { + // Do nothing +} #else +static PyTypeObject *SwigPyObject_type_global = 0; + +SWIGRUNTIME void SwigPyObject_type_global_Init(void) { + SwigPyObject_type_global = SwigPyObject_TypeOnce(); +} + SWIGRUNTIME PyTypeObject* SwigPyObject_type(void) { - static PyTypeObject *SWIG_STATIC_POINTER(type) = SwigPyObject_TypeOnce(); - return type; + assert(SwigPyObject_type_global); + return SwigPyObject_type_global; } #endif @@ -836,10 +859,16 @@ SwigPyPacked_compare(SwigPyPacked *v, SwigPyPacked *w) SWIGRUNTIME PyTypeObject* SwigPyPacked_TypeOnce(void); +static PyTypeObject* SwigPyPacked_type_global = 0; + +SWIGRUNTIME void SwigPyPacked_type_global_Init(void) { + SwigPyPacked_type_global = SwigPyPacked_TypeOnce(); +} + SWIGRUNTIME PyTypeObject* SwigPyPacked_type(void) { - static PyTypeObject *SWIG_STATIC_POINTER(type) = SwigPyPacked_TypeOnce(); - return type; + assert(SwigPyPacked_type_global); + return SwigPyPacked_type_global; } SWIGRUNTIMEINLINE int @@ -1438,11 +1467,17 @@ SWIG_Python_SetModule(swig_module_info *swig_module) { } } +static PyObject *SWIG_Python_TypeCache_global = 0; + +SWIGRUNTIME void SWIG_Python_TypeCache_global_Init(void) { + SWIG_Python_TypeCache_global = PyDict_New(); +} + /* The python cached type query */ SWIGRUNTIME PyObject * SWIG_Python_TypeCache(void) { - static PyObject *SWIG_STATIC_POINTER(cache) = PyDict_New(); - return cache; + assert(SWIG_Python_TypeCache_global); + return SWIG_Python_TypeCache_global; } SWIGRUNTIME swig_type_info * From 40ea0a960630ee26a7425bbb6d70355de031ff67 Mon Sep 17 00:00:00 2001 From: Petr Mitrichev Date: Thu, 28 Jun 2018 17:33:31 +0200 Subject: [PATCH 2/4] Add comments that explain the need for globals. --- Lib/python/pyrun.swg | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Lib/python/pyrun.swg b/Lib/python/pyrun.swg index c0d96a014..e262b657e 100644 --- a/Lib/python/pyrun.swg +++ b/Lib/python/pyrun.swg @@ -259,6 +259,9 @@ _SWIG_Py_None(void) return none; } +/* This used to be a function-local static variable, but accessing Python + * C API from inside a function-local static initializer can lead to deadlocks. + * https://github.com/swig/swig/issues/1275 */ static PyObject *SWIG_Py_None_global = 0; SWIGRUNTIME void SWIG_Py_None_global_Init(void) { @@ -506,6 +509,9 @@ SWIGRUNTIME void SwigPyObject_type_global_Init(void) { // Do nothing } #else +/* This used to be a function-local static variable, but accessing Python + * C API from inside a function-local static initializer can lead to deadlocks. + * https://github.com/swig/swig/issues/1275 */ static PyTypeObject *SwigPyObject_type_global = 0; SWIGRUNTIME void SwigPyObject_type_global_Init(void) { @@ -859,6 +865,9 @@ SwigPyPacked_compare(SwigPyPacked *v, SwigPyPacked *w) SWIGRUNTIME PyTypeObject* SwigPyPacked_TypeOnce(void); +/* This used to be a function-local static variable, but accessing Python + * C API from inside a function-local static initializer can lead to deadlocks. + * https://github.com/swig/swig/issues/1275 */ static PyTypeObject* SwigPyPacked_type_global = 0; SWIGRUNTIME void SwigPyPacked_type_global_Init(void) { @@ -1467,6 +1476,9 @@ SWIG_Python_SetModule(swig_module_info *swig_module) { } } +/* This used to be a function-local static variable, but accessing Python + * C API from inside a function-local static initializer can lead to deadlocks. + * https://github.com/swig/swig/issues/1275 */ static PyObject *SWIG_Python_TypeCache_global = 0; SWIGRUNTIME void SWIG_Python_TypeCache_global_Init(void) { From b3763c8dc8359af2dca8abc0652ec25f24c7711a Mon Sep 17 00:00:00 2001 From: William S Fulton Date: Sat, 30 Jun 2018 17:47:01 +0100 Subject: [PATCH 3/4] Remove SWIG_STATIC_POINTER (Python) It's use has been removed to discourage thread unsafe static initialisation. --- Lib/python/pyrun.swg | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/Lib/python/pyrun.swg b/Lib/python/pyrun.swg index e262b657e..51ea81306 100644 --- a/Lib/python/pyrun.swg +++ b/Lib/python/pyrun.swg @@ -210,16 +210,6 @@ SWIG_Python_UnpackTuple(PyObject *args, const char *name, Py_ssize_t min, Py_ssi /* A functor is a function object with one single object argument */ #define SWIG_Python_CallFunctor(functor, obj) PyObject_CallFunction(functor, (char*)"O", obj); -/* - Helper for static pointer initialization for both C and C++ code, for example - static PyObject *SWIG_STATIC_POINTER(MyVar) = NewSomething(...); -*/ -#ifdef __cplusplus -#define SWIG_STATIC_POINTER(var) var -#else -#define SWIG_STATIC_POINTER(var) var = 0; if (!var) var -#endif - /* ----------------------------------------------------------------------------- * Pointer declarations * ----------------------------------------------------------------------------- */ From 4b4e0180f42e992de6ff458477b70e4ec63ecb49 Mon Sep 17 00:00:00 2001 From: William S Fulton Date: Sat, 30 Jun 2018 19:18:06 +0100 Subject: [PATCH 4/4] Python initialization code tidy up I've moved the initialization of statics a little earlier on - a little safer as it is hard to follow exactly when some of these were being used, such as SWIG_Py_None which sometimes replaces Py_None. --- CHANGES.current | 5 ++++ Lib/python/pyinit.swg | 16 ++++++---- Lib/python/pyrun.swg | 70 +++++-------------------------------------- 3 files changed, 24 insertions(+), 67 deletions(-) diff --git a/CHANGES.current b/CHANGES.current index 9fb57f9f0..2a5c1c9ac 100644 --- a/CHANGES.current +++ b/CHANGES.current @@ -7,6 +7,11 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/ Version 4.0.0 (in progress) =========================== +2018-06-30: petrmitrichev + [Python] #1275 #1279 Remove function-local statics that call Python code + in order to avoid deadlocks with multi-threaded usage. These are now + file scope static variables and are initialized during module initialization. + 2018-06-15: wsfulton [Python] Fix seg fault using Python 2 when passing a Python string, containing invalid utf-8 content, to a wstring or wchar * parameter. A TypeError is thrown instead, eg: diff --git a/Lib/python/pyinit.swg b/Lib/python/pyinit.swg index ea865c066..e15eb0309 100644 --- a/Lib/python/pyinit.swg +++ b/Lib/python/pyinit.swg @@ -379,6 +379,14 @@ SWIG_init(void) { assert(metatype); #endif +#ifdef SWIG_PYTHON_BUILD_NONE + SWIG_Py_None_global = Py_BuildValue(""); + Py_DECREF(SWIG_Py_None_global); +#endif + + SWIG_This_global = SWIG_Python_str_FromChar("this"); + SWIG_Python_TypeCache_global = PyDict_New(); + /* Fix SwigMethods to carry the callback ptrs when needed */ SWIG_Python_FixMethods(SwigMethods, swig_const_table, swig_types, swig_type_initial); @@ -429,13 +437,11 @@ SWIG_init(void) { SwigPyBuiltin_AddPublicSymbol(public_interface, SwigMethods[i].ml_name); for (i = 0; swig_const_table[i].name != 0; ++i) SwigPyBuiltin_AddPublicSymbol(public_interface, swig_const_table[i].name); +#else + SwigPyObject_type_global = SwigPyObject_TypeOnce(); #endif - SWIG_Py_None_global_Init(); - SwigPyObject_type_global_Init(); - SwigPyPacked_type_global_Init(); - SWIG_Python_TypeCache_global_Init(); - + SwigPyPacked_type_global = SwigPyPacked_TypeOnce(); SWIG_InstallConstants(d,swig_const_table); %} diff --git a/Lib/python/pyrun.swg b/Lib/python/pyrun.swg index 51ea81306..5ad5713fd 100644 --- a/Lib/python/pyrun.swg +++ b/Lib/python/pyrun.swg @@ -241,42 +241,20 @@ extern "C" { # undef Py_None # define Py_None SWIG_Py_None() # endif -SWIGRUNTIMEINLINE PyObject * -_SWIG_Py_None(void) -{ - PyObject *none = Py_BuildValue(""); - Py_DECREF(none); - return none; -} -/* This used to be a function-local static variable, but accessing Python - * C API from inside a function-local static initializer can lead to deadlocks. - * https://github.com/swig/swig/issues/1275 */ static PyObject *SWIG_Py_None_global = 0; -SWIGRUNTIME void SWIG_Py_None_global_Init(void) { - SWIG_Py_None_global = _SWIG_Py_None(); -} - SWIGRUNTIME PyObject * -SWIG_Py_None(void) -{ +SWIG_Py_None(void) { assert(SWIG_Py_None_global); return SWIG_Py_None_global; } - -#else -SWIGRUNTIME void SWIG_Py_None_global_Init(void) -{ - // Do nothing -} #endif /* The python void return value */ SWIGRUNTIMEINLINE PyObject * -SWIG_Py_Void(void) -{ +SWIG_Py_Void(void) { PyObject *none = Py_None; Py_INCREF(none); return none; @@ -485,6 +463,7 @@ SWIGRUNTIME PyTypeObject* SwigPyObject_TypeOnce(void); #ifdef SWIGPYTHON_BUILTIN static swig_type_info *SwigPyObject_stype = 0; + SWIGRUNTIME PyTypeObject* SwigPyObject_type(void) { SwigPyClientData *cd; @@ -494,20 +473,9 @@ SwigPyObject_type(void) { assert(cd->pytype); return cd->pytype; } - -SWIGRUNTIME void SwigPyObject_type_global_Init(void) { - // Do nothing -} #else -/* This used to be a function-local static variable, but accessing Python - * C API from inside a function-local static initializer can lead to deadlocks. - * https://github.com/swig/swig/issues/1275 */ static PyTypeObject *SwigPyObject_type_global = 0; -SWIGRUNTIME void SwigPyObject_type_global_Init(void) { - SwigPyObject_type_global = SwigPyObject_TypeOnce(); -} - SWIGRUNTIME PyTypeObject* SwigPyObject_type(void) { assert(SwigPyObject_type_global); @@ -855,15 +823,8 @@ SwigPyPacked_compare(SwigPyPacked *v, SwigPyPacked *w) SWIGRUNTIME PyTypeObject* SwigPyPacked_TypeOnce(void); -/* This used to be a function-local static variable, but accessing Python - * C API from inside a function-local static initializer can lead to deadlocks. - * https://github.com/swig/swig/issues/1275 */ static PyTypeObject* SwigPyPacked_type_global = 0; -SWIGRUNTIME void SwigPyPacked_type_global_Init(void) { - SwigPyPacked_type_global = SwigPyPacked_TypeOnce(); -} - SWIGRUNTIME PyTypeObject* SwigPyPacked_type(void) { assert(SwigPyPacked_type_global); @@ -1004,20 +965,12 @@ SwigPyPacked_UnpackData(PyObject *obj, void *ptr, size_t size) * pointers/data manipulation * ----------------------------------------------------------------------------- */ -SWIGRUNTIMEINLINE PyObject * -_SWIG_This(void) -{ - return SWIG_Python_str_FromChar("this"); -} - -static PyObject *swig_this = NULL; +static PyObject *SWIG_This_global = NULL; SWIGRUNTIME PyObject * -SWIG_This(void) -{ - if (swig_this == NULL) - swig_this = _SWIG_This(); - return swig_this; +SWIG_This(void) { + assert(SWIG_This_global); + return SWIG_This_global; } /* #define SWIG_PYTHON_SLOW_GETSET_THIS */ @@ -1446,7 +1399,7 @@ SWIG_Python_DestroyModule(PyObject *obj) } } Py_DECREF(SWIG_This()); - swig_this = NULL; + SWIG_This_global = NULL; } SWIGRUNTIME void @@ -1466,15 +1419,8 @@ SWIG_Python_SetModule(swig_module_info *swig_module) { } } -/* This used to be a function-local static variable, but accessing Python - * C API from inside a function-local static initializer can lead to deadlocks. - * https://github.com/swig/swig/issues/1275 */ static PyObject *SWIG_Python_TypeCache_global = 0; -SWIGRUNTIME void SWIG_Python_TypeCache_global_Init(void) { - SWIG_Python_TypeCache_global = PyDict_New(); -} - /* The python cached type query */ SWIGRUNTIME PyObject * SWIG_Python_TypeCache(void) {