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

I support this idea too.  I had a PR that got into 4.0 for essentially the
same issue on containers like std::vector that might be of interest:

https://github.com/swig/swig/pull/1234

Strategy there was to point to the parent wrapper from the child wrapper to
tie their GC lifetimes together.

-Jake

On Wed, Jul 22, 2020, 7:42 AM Terry Barnaby <[email protected]> wrote:

> Hi,
>
> Yes, it is a very simple patch that just increments the reference counter
> on the SWIG wrapped C++ object when a member of it is returned via a
> pointer/reference and decrements that reference when the returned member is
> finally deleted. The idea would be to implement this as a SWIG feature than
> could be turned on for an individual class/struct or the whole library. I
> believe it would be of use if quite a few cases, but as said, I may be
> missing something.
>
> Terry
> On 22/07/2020 10:57, D Haley wrote:
>
> Dear List,
>
> I would like to support the demand side of things for this, I maintain a
> small library that provide C++->python using swig, and we've had troubles
> with library users having memory corruption when using python. Our
> workaround has been to discourage the inadvertent creation of temporary
> variables, but it seems a poor solution.
>
> As a library maintainer, it sounds like (and I've not gone through the
> patch in detail), that this reference counting approach would help minimise
> the chance of corruption by early collection?
>
> Thanks.
> On 22/07/2020 10:10, Terry Barnaby wrote:
>
> Hi,
>
> Anyone any views on this ?
> I am happy to try and create a patch to add this as a "feature" enabled
> function if people think it would be worthwhile and would be included in
> the system. It certainly seems to be a good idea from our libraries point
> of view.
>
> Terry
> On 08/07/2020 06:57, Terry Barnaby wrote:
>
> Hi William,
>
> So do you think this idea of incrementing the reference counter on the C++
> object whose member you are returning by reference is reasonable and would
> be a good addition to SWIG as a "feature" that can be enabled ? If so I can
> look at producing a proper patch for adding this (have to work out how the
> features system works first). As I don't know the SWIG code in depth it
> would need a good look at once I have done it.
>
> Terry
> On 01/07/2020 11:50, Terry Barnaby wrote:
>
> Hi William,
>
> Enclosed is my very experimental, basic and not for use example of the
> sort of thing I was thinking of.
> This is Python specific has quite a few hard coded things that won't work
> in all cases etc etc and assumes all members are returned as a pointer,
> which is incorrect.
>
> But with a lot more work, maybe something like this could be made to work
> when enabled using a particular "feature" setting ?
>
> Basically what it should do is whenever a pointer to a C++ objects member
> is returned, it will increment the reference counter on the SWIG wrapper
> for the container object and set a pointer to this container on the
> returned member pointer PyObject. Then when the member's pointer PyObject
> is deleted, it will decrement the reference counter on the overall C++
> container object so it can be deleted.
>
> I'm probably missing a few things here, but I wonder if this sort of
> mechanism could handle a lot of normal situations when pointers to C++
> members are returned ?
>
> Terry
> On 01/07/2020 06:44, Terry Barnaby wrote:
>
> Hi William,
>
> Thanks for the information. In the case of the class library I'm wrapping
> that would end up with an awful amount of code in SWIG to handle every case
> where this occurs. Most, if not all of the classes/structs I am
> encapsulating use other classes/structs as members down multiple levels and
> changes would be needed to this code for every library update. It might end
> up it would be easier not to use SWIG :)
>
> Yes, memory management, pass by value, reference/pointer etc is tough, but
> I am surprised that SWIG doesn't handle this, at least for Python, with a
> user configurable "feature" or all of the time. From what I can see for C++
> -> Python at least it would be possible to add the code, as given in the
> references you give, by default whenever a reference to a C++ object
> members data is being returned as a pointer. I think it would be a
> generally the case that if you return a reference to a C++ objects member
> you would want the overall C++ object to hang around while that pointer is
> in existence.
>
> Yes, this would reduce efficiency, but if instead of the methods suggested
> a simpler and more efficient Py_INCREF() and Py_DECREF() mechanism on the
> PyObject* to the parent object could be used the overhead should be pretty
> small. I have tried a simple implementation of this by modifying SWIG to do
> the Py_INCREF() and it appears to work well (although it is a hack as I
> don't understand the SWIG internals well enough) and I don't know how to
> call Py_DECREF() efficiently so I am not doing this. I am assuming it would
> be possible to modify the SWIG C++ wrapper object to have the extra
> container objects pointer so its reference count can be decremented but I
> need to try and understand the SWIG code better.
>
> If you think something like this might be feasible and useful, I will have
> a look at the SWIG code a bit more in my limited spare time.
>
> Terry
> On 28/06/2020 13:32, William S Fulton wrote:
>
> Hi Terry
>
> This is one of the most horrible problem areas mixing languages, that is,
> memory management. Here is some background reading:
>
>
> http://www.swig.org/Doc4.0/Python.html#Python_memory_management_member_variables
> https://github.com/swig/swig/issues/945
> https://github.com/swig/swig/pull/1234
>
> Below is one way you can fix this (it's a variation of returning members
> by reference documented in the 1st link above). Note that SWIG uses
> pointers for accessing non-primitive member variables, see
> http://www.swig.org/Doc4.0/SWIG.html#SWIG_structure_data_members.
>
> %fragment("member_parent_attribute_init", "init") {
>   // thread safe initialization
>   member_parent_attribute();
> }
>
> %fragment("member_parent_attribute", "header",
> fragment="member_parent_attribute_init") {
>   static PyObject* member_parent_attribute() {
>     static PyObject* attr = SWIG_Python_str_FromChar("__member_parent");
>     return attr;
>   }
> }
> %typemap(ret, fragment="member_parent_attribute", noblock=1) ListA
> *TypeB::list1 %{
>   PyObject_SetAttr($result, member_parent_attribute(), $self);
> %}
>
> William
>
> On Thu, 25 Jun 2020 at 14:58, Terry Barnaby <[email protected]
> <mailto:[email protected]> <[email protected]>> wrote:
>
>     I have had a quick look at the SWIG code.
>
>     It seems that it would be possible, and desirable to do a
>     Py_INCREF() on
>     the PyObject* object that is wrapping the C++ object which you are
>     returning a pointer to the member from. This would hold the
>     whole 'C++'
>     object in memory so that returning a pointer to one of this
>     objects
>     members would be ok. The PyObject* returned for the member
>     would have to
>     have a pointer to its parents PyObject* and do a Py_DECREF() on
>     the
>     parent object when it is finally deleted so that the original
>     C++ object
>     gets cleaned up eventually.
>
>     I don't know the SWIG code to know if this is really possible
>     and if
>     people think this is a reasonable SWIG feature that could be
>     added ?
>     At the moment a user of your SWIG generated Python API gets a
>     pointer to
>     a destroyed object which is not good.
>
>     Terry
>     On 24/06/2020 10:04, Terry Barnaby wrote:
>     > I have a class library that has List objects as members of
>     the class.
>     > If I use this class in a Python function and try to return
>     one of
>     > these more complex members all hell breaks loose. It appears
>     the SWIG
>     > generated wrapper returns a pointer to the objects member
>     rather than
>     > making a copy of it when returning the member from the function.
>     >
>     > So I have something like:
>     >
>     > class TypeB {
>     >
>     > public:
>     >
>     >     List<TypeA> list1;
>     >
>     > };
>     >
>     > If in Python I have a function like the following, the
>     returned item
>     > is effectively (SWIG wrapped) a pointer the b objects member
>     and b
>     > disappears when the function returns.
>     >
>     > def func1():
>     >
>     >     b = TypeB();
>     >
>     >     b.list1.append(TypeA());
>     >
>     >     return b.list1;
>     >
>     > Now I can use "return ListTypeA(b.list1)", where ListTypeA
>     has been
>     > declared using a %template declaration such that the copy
>     constructor
>     > is called to explicitly make a copy of the list, but is there
>     a better
>     > way of handing this ?
>     >
>     > I know this is difficult to handle, but perhaps something like a
>     > %template(return) to add code when returning a type in a Python
>     > function somehow or maybe the overall objects reference
>     counter can be
>     > incremented ?
>     >
>     >
>     >
>     > _______________________________________________
>     > Swig-user mailing list
>     > [email protected]
>     <mailto:[email protected]>
> <[email protected]>
>     > https://lists.sourceforge.net/lists/listinfo/swig-user
>
>
>
>
>     _______________________________________________
>     Swig-user mailing list
>     [email protected]
>     <mailto:[email protected]>
> <[email protected]>
>     https://lists.sourceforge.net/lists/listinfo/swig-user
>
>
>
>
>
>
>
>
> _______________________________________________
> Swig-user mailing [email protected]://lists.sourceforge.net/lists/listinfo/swig-user
>
>
>
> _______________________________________________
> Swig-user mailing [email protected]://lists.sourceforge.net/lists/listinfo/swig-user
>
>
> _______________________________________________
> Swig-user mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/swig-user
>

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