From de6b433cb178c2e4b7b2f3c7dfc02a90ddedfccf Mon Sep 17 00:00:00 2001 From: Thomas Maslach Date: Tue, 23 Sep 2014 20:22:20 +0100 Subject: [PATCH] Fix Python crash when using -threads iterating containers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also fixes li_std_vector_enum testcase when run with -threads. Patch supplied on swig-devel mailing list on 12 Sep with details... ============================================== I just wanted to mention that I found a crash issue in bug.. I am using SWIG 2.0.11 with python and have –threads enabled. I have a C++ std::vector that I instantiate in SWIG with %template. I also have a method in a class that returns this vector. I also include std_vector.i, btw.. When I iterate like so: children = Action.getActionList() for child in children: pass Everything is fine.. When I iterate like this: for child in Action.getActionList() pass Product crashes. The problem is the following. This code gets called first: SWIGINTERN PyObject *_wrap_delete_SwigPyIterator(PyObject *SWIGUNUSEDPARM(self), PyObject *args) { PyObject *resultobj = 0; swig::SwigPyIterator *arg1 = (swig::SwigPyIterator *) 0 ; void *argp1 = 0 ; int res1 = 0 ; PyObject * obj0 = 0 ; if(!PyArg_UnpackTuple(args,(char *)"delete_SwigPyIterator",1,1,&obj0)) SWIG_fail; res1 = SWIG_ConvertPtr(obj0, &argp1,SWIGTYPE_p_swig__SwigPyIterator, SWIG_POINTER_DISOWN | 0 ); if (!SWIG_IsOK(res1)) { SWIG_exception_fail(SWIG_ArgError(res1), "in method '" "delete_SwigPyIterator" "', argument " "1"" of type '" "swig::SwigPyIterator *""'"); } arg1 = reinterpret_cast< swig::SwigPyIterator * >(argp1); { SWIG_PYTHON_THREAD_BEGIN_ALLOW; delete arg1; SWIG_PYTHON_THREAD_END_ALLOW; } resultobj = SWIG_Py_Void(); return resultobj; fail: return NULL; } Note the SWIG_PYTHON_THREAD_BEGIN_ALLOW/END_ALLOW. In between those two statements, we delete arg1. That in turn will eventually end up in this code: namespace swig { class SwigPtr_PyObject { protected: PyObject *_obj; public: … snip! … ~SwigPtr_PyObject() { Py_XDECREF(_obj); } Uh-oh! We call Py_XDECREF when we aren’t supposed to because we are in a SWIG_PYTHON_THREAD_BEGIN_ALLOW/END_ALLOW section! This takes care of the issue: namespace swig { class SwigPtr_PyObject { protected: PyObject *_obj; public: … snip! … ~SwigPtr_PyObject() { SWIG_PYTHON_THREAD_BEGIN_BLOCK; Py_XDECREF(_obj); SWIG_PYTHON_THREAD_END_BLOCK; } There are several other methods in this class that use the Python API, but don’t have the BEGIN/END block defined. I’m not sure if they are required for all of them, but I believe they are.. I have attached a modified pyclasses.swg with what I believe are the correct changes. This code is from 2.0.11, but as far as I can tell, it’s the same as what is in 3.0.2… Apologies for not doing more here (making/running tests, getting it in the code repository, etc..), but I’m under some pressure to get some unrelated things done… --- CHANGES.current | 4 ++ .../test-suite/python/python_threads_runme.py | 10 +++++ Examples/test-suite/python_threads.i | 40 +++++++++++++++++++ Lib/python/pyclasses.swg | 8 ++++ 4 files changed, 62 insertions(+) create mode 100644 Examples/test-suite/python/python_threads_runme.py create mode 100644 Examples/test-suite/python_threads.i diff --git a/CHANGES.current b/CHANGES.current index d5dcb05f6..781b21c1c 100644 --- a/CHANGES.current +++ b/CHANGES.current @@ -5,6 +5,10 @@ See the RELEASENOTES file for a summary of changes in each release. Version 3.0.3 (in progress) =========================== +2014-09-23: wsfulton + [Python] Add patch from Thomas Maslach to fix crash in wrappers when using -threads in + the STL iterators (SwigPyIterator destructor). + 2014-09-17: wsfulton [C#] Merge patch #229 from contre - Add bool array types to arrays_csharp.i diff --git a/Examples/test-suite/python/python_threads_runme.py b/Examples/test-suite/python/python_threads_runme.py new file mode 100644 index 000000000..d00e2458f --- /dev/null +++ b/Examples/test-suite/python/python_threads_runme.py @@ -0,0 +1,10 @@ +from python_threads import * + +action = ActionGroup() +count = 1 +for child in action.GetActionList(): + if child.val != count: + raise RuntimeError("Expected: " + str(count) + " got: " + str(child.val)) + count = count + 1 + +# Was seg faulting at the end here diff --git a/Examples/test-suite/python_threads.i b/Examples/test-suite/python_threads.i new file mode 100644 index 000000000..08eddef11 --- /dev/null +++ b/Examples/test-suite/python_threads.i @@ -0,0 +1,40 @@ +%module(threads=1) python_threads + +%include + +%inline %{ +struct Action { + int val; + Action(int val = 0) : val(val) {} +}; +%} + +%template(VectorActionPtr) std::vector; + +%inline %{ +#include +#include +template struct myStlVector : public std::vector { +}; +typedef myStlVector ActionList; + +%} + +%template(ActionList) myStlVector; + +%inline %{ +class ActionGroup +{ +public: + ActionList &GetActionList () const { + static ActionList list; + list.push_back(new Action(1)); + list.push_back(new Action(2)); + list.push_back(new Action(3)); + list.push_back(new Action(4)); + return list; + } +}; +%} + + diff --git a/Lib/python/pyclasses.swg b/Lib/python/pyclasses.swg index b73ebdbb8..9d6299ff1 100644 --- a/Lib/python/pyclasses.swg +++ b/Lib/python/pyclasses.swg @@ -72,27 +72,35 @@ namespace swig { SwigPtr_PyObject(const SwigPtr_PyObject& item) : _obj(item._obj) { + SWIG_PYTHON_THREAD_BEGIN_BLOCK; Py_XINCREF(_obj); + SWIG_PYTHON_THREAD_END_BLOCK; } SwigPtr_PyObject(PyObject *obj, bool initial_ref = true) :_obj(obj) { if (initial_ref) { + SWIG_PYTHON_THREAD_BEGIN_BLOCK; Py_XINCREF(_obj); + SWIG_PYTHON_THREAD_END_BLOCK; } } SwigPtr_PyObject & operator=(const SwigPtr_PyObject& item) { + SWIG_PYTHON_THREAD_BEGIN_BLOCK; Py_XINCREF(item._obj); Py_XDECREF(_obj); _obj = item._obj; + SWIG_PYTHON_THREAD_END_BLOCK; return *this; } ~SwigPtr_PyObject() { + SWIG_PYTHON_THREAD_BEGIN_BLOCK; Py_XDECREF(_obj); + SWIG_PYTHON_THREAD_END_BLOCK; } operator PyObject *() const