Fix Python crash when using -threads iterating containers

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

@@ -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
 

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

Examples/test-suite/python_threads.i

@@ -0,0 +1,40 @@
+%module(threads=1) python_threads
+
+%include <std_vector.i>
+
+%inline %{
+struct Action {
+  int val;
+  Action(int val = 0) : val(val) {}
+};
+%}
+
+%template(VectorActionPtr) std::vector<Action *>;
+
+%inline %{
+#include <vector>
+#include <iostream>
+template <typename T> struct myStlVector : public std::vector<T> {
+};
+typedef myStlVector <Action *> ActionList;
+
+%}
+
+%template(ActionList) myStlVector<Action *>;
+
+%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;
+  }
+};
+%}
+
+

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