Re: C++ to Python and returning members of a C++ object

Hi Jake,

Ok, I will look at doing a pull once it is in a state to do so.

I was intending to only use the reference counting system for member 
gets for now and only when the containerReference feature is enabled so 
it won't affect any other code (apart from the slight increase in memory 
usage).
However, I can look at using if for the reference counting in your 
container code if people think it is worth going further with it ?

I'm not sure on your self vs args change, I am missing something here, 
are you using some particular swig "features" when generating the 
wrapper? A typical *_get function on my wrapper looks like it needs the 
args, the code looks like:
> SWIGINTERN PyObject *_wrap_ClassB_b_get(PyObject 
> *SWIGUNUSEDPARM(self), PyObject *args) {
>   PyObject *resultobj = 0;
>   ClassB *arg1 = (ClassB *) 0 ;
>   void *argp1 = 0 ;
>   int res1 = 0 ;
>   PyObject *swig_obj[1] ;
>   ClassA *result = 0 ;
>
>   if (!args) SWIG_fail;
>   swig_obj[0] = args;
>   res1 = SWIG_ConvertPtr(swig_obj[0], &argp1,SWIGTYPE_p_ClassB, 0 |  0 );
>   if (!SWIG_IsOK(res1)) {
>     SWIG_exception_fail(SWIG_ArgError(res1), "in method '" 
> "ClassB_b_get" "', argument " "1"" of type '" "ClassB *""'");
>   }
>   arg1 = reinterpret_cast< ClassB * >(argp1);
>   result = (ClassA *)& ((arg1)->b);
>   resultobj = SWIG_NewPointerObj(SWIG_as_voidptr(result), 
> SWIGTYPE_p_ClassA, 0 |  0 );
>   SwigContainerReference(args, resultobj); // Increment reference 
> count of the container object and add a pointer to the container to 
> this in the returned object
>   return resultobj;
> fail:
>   return NULL;
> }
My current patch and example is attached.

Terry
On 22/09/2020 21:49, Jake wrote:
> Terry,
>
> You should open a pull request on Github with your proposed change.  
> William or another maintainer will have to accept it to get in.
>
> I'm not a maintainer, but I would recommend the following:
>
> Replace my implementation of the container owner function by your own, 
> but keep the template specialization so the container code works as 
> before.
> In your insertion points, use swig::pointer_category as the template 
> argument so the ref-counting specialization is called.
>
> Yes, I replaced 'args' with 'self' where you showed. 'self' is the 
> wrapper for the parent instance, 'args' is a tuple of function 
> arguments to be unpacked.
>
> When you have in Python:
>
> m = Mixed()
> o = m.obj
>
> And Mixed.obj is some wrapped type (e.g. not a string or other value 
> that gets copied), look at the generated code:
>
> 6903 SWIGINTERN PyObject *_wrap_Mixed_obj_get(PyObject *self, PyObject 
> *args) {
>
> 6904 PyObject *resultobj = 0;
>
> 6905 Mixed *arg1 = (Mixed *) 0;
>
> 6906 void*argp1 = 0;
>
> 6907 intres1 = 0;
>
> 6908 PyObject *swig_obj[1] ;
>
> 6909 MyObject *result = 0;
>
> 6910
>
> 6911 if(!SWIG_Python_UnpackTuple(args, "Mixed_obj_get", 0, 0, 0)) 
> SWIG_fail;
>
> 6912 res1 = SWIG_ConvertPtr(self, &argp1,SWIGTYPE_p_Mixed, 0|0);
>
> 6913 if(!SWIG_IsOK(res1)) {
>
> 6914 SWIG_exception_fail(SWIG_ArgError(res1), "in method 
> '""Mixed_obj_get""', argument ""1"" of type '""Mixed *""'");
>
> 6915 }
>
> 6916 arg1 = reinterpret_cast< Mixed * >(argp1);
>
> 6917 result = (MyObject *)& ((arg1)->obj);
>
> 6918 resultobj = SWIG_NewPointerObj(SWIG_as_voidptr(result), 
> SWIGTYPE_p_MyObject, 0|0);
>
> 6919 SwigContainerReference(args, resultobj); // Increment reference 
> count of the container object and add a pointer to the container to 
> this in the returned object
>
>
> Without that change I was getting crashes.
>
> -Jake
>
>
>
> On Tue, Sep 22, 2020 at 10:28 AM Terry Barnaby <[email protected] 
> <mailto:[email protected]>> wrote:
>
>     Hi Jake,
>
>     I have been away from this for a while with other work.
>
>     You stated:
>     On 19/08/2020 09:36, Terry Barnaby wrote:
>     > It segfaults on pointer-category objects because you should be
>     passing
>     > "self" as the first argument not "args", I corrected that for the
>     > benchmarks above.
>
>     Not sure what you mean here, the only place I use "args " is in:
>
>         Printf(f->code, "SwigContainerReference(args, resultobj); //
>     Increment reference count of the container object and add a
>     pointer to
>     the container to this in the returned object\n");
>
>     And this seems correct, certainly I can't use "self" in place of
>     "args"
>     here as the code won't compile. May I ask what change you made here ?
>
>     I'm not sure where the SWIG group want to go on this. For the
>     projects
>     I'm involved in I need this fix to eliminate the memory faults when
>     returning C++ members in Python so I will be creating my own local
>     version of SWIG, with my patch applied, for now. But I am keen to try
>     and get something like this included in the main tree if possible.
>
>     Terry
>

_______________________________________________
Swig-user mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/swig-user

diff --git a/Lib/python/pyrun.swg b/Lib/python/pyrun.swg
index 63ff82ff8..3635ed846 100644
--- a/Lib/python/pyrun.swg
+++ b/Lib/python/pyrun.swg
@@ -330,8 +330,34 @@ typedef struct {
 #ifdef SWIGPYTHON_BUILTIN
   PyObject *dict;
 #endif
+  PyObject *objContainer;
 } SwigPyObject;
 
+// This is rough experimental code to increase the reference count of the PyObject that wraps a C++ object when a pointer
+// to one of the C++ object's data members is returned in a PyObject. It makes sure the parent C++ object remains in
+// existance whilst a pointer to one of its members is in use.
+// The associated code in cwrap.c and python.cxx assumes all members are returned as a pointer, which is incorrect.
+SWIGRUNTIME SwigPyObject *SWIG_Python_GetSwigThis(PyObject *pyobj);
+
+// This is used when a PyObject pointing ot a C++ objects member is returned
+void SwigContainerReference(PyObject* container, PyObject* member){
+	SwigPyObject*	m = SWIG_Python_GetSwigThis(member);
+	
+	// printf("SwigContainerReference: %p %p %p\n", container, member, m);
+	if(m){
+		Py_INCREF(container);
+		m->objContainer = container;
+	}
+}
+
+// This is called when a PyObject wrapping a C++ object is destroyed
+void SwigContainerDereference(SwigPyObject* obj){
+	//printf("SwigContainerDereference: %p\n", obj);
+	if(obj && obj->objContainer){
+		//printf("SwigContainerDereference: objContainer: %p\n", obj->objContainer);
+		Py_DECREF(obj->objContainer);
+	}
+}
 
 #ifdef SWIGPYTHON_BUILTIN
 
@@ -524,6 +550,9 @@ SwigPyObject_dealloc(PyObject *v)
   } 
   Py_XDECREF(next);
   PyObject_DEL(v);
+
+  // If this object is a reference to a C++ container object, decrement the reference count on the container object
+  SwigContainerDereference(sobj);
 }
 
 SWIGRUNTIME PyObject* 
@@ -740,6 +769,7 @@ SwigPyObject_New(void *ptr, swig_type_info *ty, int own)
     sobj->ty   = ty;
     sobj->own  = own;
     sobj->next = 0;
+    sobj->objContainer = 0;	// Pointer to container object that this object is a member of
   }
   return (PyObject *)sobj;
 }
diff --git a/Source/Modules/python.cxx b/Source/Modules/python.cxx
index c8c45df35..9250b17c9 100644
--- a/Source/Modules/python.cxx
+++ b/Source/Modules/python.cxx
@@ -3144,6 +3144,10 @@ public:
 #else
       Printf(f->code, "%s\n", tm);
 #endif
+
+      if(parent && GetFlag(parent,"feature:containerReference") && Getattr(n, "memberget"))
+          Printf(f->code, "SwigContainerReference(args, resultobj); // Increment reference count of the container object and add a pointer to the container to this in the returned object\n");
+
       Delete(tm);
     } else {
       Swig_warning(WARN_TYPEMAP_OUT_UNDEF, input_file, line_number, "Unable to use return type %s in function %s.\n", SwigType_str(d, 0), name);