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

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] 
> <mailto:[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]>
>>>>>>>     <mailto:[email protected]> <mailto:[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]>
>>>>>>>     <mailto:[email protected]>
>>>>>>>     <mailto:[email protected]>
>>>>>>>         > https://lists.sourceforge.net/lists/listinfo/swig-user
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>     _______________________________________________
>>>>>>>         Swig-user mailing list
>>>>>>>     [email protected]
>>>>>>>     <mailto:[email protected]>
>>>>>>>     <mailto:[email protected]>
>>>>>>>     <mailto:[email protected]>
>>>>>>>     https://lists.sourceforge.net/lists/listinfo/swig-user
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>>
>>>     _______________________________________________
>>>     Swig-user mailing list
>>>     [email protected]  <mailto:[email protected]>
>>>     https://lists.sourceforge.net/lists/listinfo/swig-user
>>
>>
>>     _______________________________________________
>>     Swig-user mailing list
>>     [email protected]  <mailto:[email protected]>
>>     https://lists.sourceforge.net/lists/listinfo/swig-user
>
>
>     _______________________________________________
>     Swig-user mailing list
>     [email protected]
>     <mailto:[email protected]>
>     https://lists.sourceforge.net/lists/listinfo/swig-user
>

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