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

Hi Terry

I don't know why you have not based your solutions on the approaches
mentioned in my original response. One of those is the std::vector solution
that Jake also brought up. What I think might work is, instead of a new
feature, is a new typemap that is only used when wrapping 'getter' member
variables. Let's call it say memberout. It will contain the typemap code
that I posted originally and only be generated when wrapping member
variable getters. I'd need to think how to make it work only when needed,
which I think is for all member types stored by value that are a SWIG
wrapped proxy class type.

William

On Wed, 22 Jul 2020 at 13:45, Terry Barnaby <[email protected]> wrote:

> In effect, this is what my experimental patch does at the SWIG C++
> wrapping level. It adds a PyObject* to the members SwigPyObject that is set
> to point to the container object that the members SwigPyObject is
> referencing. This PyObject* pointer is set on a members SwigPyObject object
> when a pointer/reference to a member is created as well as the container
> object having its reference counter incremented. When the members
> SwigPyObject is deleted it will use the PyObject* to decrement the
> container objects reference counter if the PyObject* is actually set. There
> is the addition of the pointer to every SwigPyObject and the call to the
> reference counter increment/decrement but that is all so it seems pretty
> efficient if this method is ok.
>
> It is simple, but I am unsure of its implications in the wider SWIG
> codebase as I don't have much knowledge of this. So any comments on this
> approach welcome.
> If ok, I will have to work out how to use the "feature" functionality of
> SWIG to implement it properly.
>
> Terry
>
> On 22/07/2020 13:00, Jake wrote:
>
> 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
>

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