PyBindGen

Reference counted objects and std containers

Asked by Michael Brown

Hi,

I have two questions and a contribution regarding using PyBindGen.

Q 1) ReferenceCountingMethodsPolicy seems to show that the peekref parameter is optional. That's good, because our project does not have such a capability. However, when I pass only incref_method and decref_method I get the following.

Code generation and compilation gives me this:
static int
PyWorld__tp_traverse(PyWorld *self, visitproc visit, void *arg)
{
    Py_VISIT(self->inst_dict);

    if (self->obj && typeid(*self->obj) == typeid(PyWorld__PythonHelper) && self->obj->None() == 1)
        Py_VISIT((PyObject *) self);

    return 0;
}
Which results in this error:
    world-binding.cpp: In function ‘int PyWorld__tp_traverse(PyWorld*, int (*)(PyObject*, void*), void*)’:
    world-binding.cpp:458:89: error: ‘class World’ has no member named ‘None’

So my question is, why would it check for unique ownership. I know it's to do with Python garbage collection, but I can't think of a scenario where the check is useful here. Also, how can I fix this (without hacking our project to include a peekref.. I don't have that level of access).

Q 2) I am using add_container to support properties of type std::vector<World*>, which works, but because it wraps the returned vector I am left with a lot of pointers that haven't had their reference count incremented living outside of the original container. Should the original object drop its references, I'll have stale pointers. Is there a way to have PyBindGen return a python copy of the list, with object wrapper entries instead, so that normal memory management rules apply, and reference counts have been incremented, and ultimately will be decremented when the list is deleted?

Contribution) I have copied and modified the std::string support classes for Glib::ustring. I'd like to give these back to the project, as I think that they are generally useful. How do I do this?

Any questions, if I have missed something etc, please ask.

And a final note, I have searched google silly, gone over the docs many times and tried to understand the source code. I am not asking these questions without having spent a long time trying to find the answers first.

Many thanks,
Michael.

My example class description.

mod = pybindgen.Module('MyModule')

mempolicy = pybindgen.cppclass.ReferenceCountingMethodsPolicy(incref_method='reference', decref_method='unreference')

mod.add_include('"world.h"')

klass = mod.add_class('World', memory_policy=mempolicy, automatic_type_narrowing=True, allow_subclassing=True)
mod.add_container('std::vector<World*>', 'World*', 'vector')

klass.add_constructor([])

klass.add_method('do_hello', None, [param('Glib::ustring&', 'val')], is_virtual=True, custom_name='_do_hello', visibility='protected')
klass.add_method('do_other', retval('World *', caller_owns_return=True), [], is_virtual=True, custom_name='_do_other', visibility='protected')
klass.add_instance_attribute('name', retval('Glib::ustring'), getter='name', setter='set_name')
klass.add_instance_attribute('sizes', retval('std::vector<World*>'), getter='sizes', setter='set_sizes')

klass.add_method('hello', None, [param('Glib::ustring&', 'val')], custom_name='hello', visibility='public')
klass.add_method('other', retval('World *', caller_owns_return=True), [], custom_name='other', visibility='public')

mod.generate(sys.stdout)

Question information

Language:
English Edit question
Status:
Solved
For:
PyBindGen Edit question
Assignee:
No assignee Edit question
Solved by:
Michael Brown
Solved:
Last query:
Last reply:
Revision history for this message
Gustavo Carneiro (gjc) said : 		#1

Sorry, the None error is a bug.
1.
Without the peekref method, the python wrapper IncRef's the C++ obj, but not the other way around. Thus, if you pass the C++ object to some C++ function that IncRefs it, then the Python object goes out of scope, it is released. Thus, the Python wrapper is released, but the C++ object isn't. If the C++ object has a virtual method implemented in Python, we have to create a brand new wrapper when the virtual method is invoked. This brand new Python wrapper will have a new __dict__, so any instance variables you may have set previously have been cleared.

With PeekRef available, the wrapper Refs the C++ object, and the C++ object conceptually Refs the Python wrapper back. Then when the Python GC kicks in (it periodically checks for reference cycles), we check to see if the Python wrapper is the last one to Ref the C++ object (PeekRef()==1), and if so we release both wrapper and C++ object.

2.
Not sure I understand the std::vector<World*> problem. Does World have reference counting? If so, I think there is a way to make pybindgen use it in the container, I can check later.

3. Patches should go into bug reports. Thanks.

Revision history for this message
Michael Brown (mbrown-7) said : 		#2

Thanks for the thorough reply.

1) After I read the answer a few times, and finally managed to bend my mind around it, I understand the dilemma. It is indeed a serious problem. We, for example, will be exposing a C++ widget hierarchy to python, and will frequently see python sub-classes existing only within C++ containers. As I understand, this will almost certainly cause the issue you describe.

We are using glibmm as our base object system, and it inherits its reference counting semantics from glib, which documents the ref_count field of gobject as private and must not be touched. No methods exist to get the value either. I think that this makes sense in a threaded environment, where the value read is not necessarily correct if another thread is mutating it. I am trying desperately to think of another way to solve this problem without breaking gobject rules.

2) Yes, reference counted. It would be great if it used the reference counting.

3) I'll sort out a patch shortly.

Revision history for this message
Gustavo Carneiro (gjc) said : 		#3

1)

Well, in the past pygtk used the "peekref" approach. I copied the technique into pybindgen, actually ;-)

Recently, they use the concept of "toggle references" (g_object_add_toggle_ref) to allow language bindings based on wrappers. Pybindgen does not support this atm.

2) I think if you declare the type used in the container with caller_owns_return=False, it automatically increfs the object before adding it to the container. I'm not 100% sure, please check in the generated code:

mod.add_container('std::vector<World*>', retval('World*', calller_owns_return=False), 'vector')

Revision history for this message
Michael Brown (mbrown-7) said : 		#4

1) I have now downloaded pygtk, pygobject and poked around inside glibmm. I conclude, that whilst they are right to use this toggle reference technique, it's very gobject specific and would be difficult for a technology agnostic project such as PyBindGen to support. Worse though, the Glibmm maintainers in their wisdom have decided to not expose this functionality. Perhaps because Glibmm itself is a wrapper library, and the functionality was provided to support wrappers, not to be wrapped. Suddenly hacking a peek ref into Glibmm looks like a good idea.

2) I tried caller_owns_return=False, and can't see any material change in the generated code for the getter. However I did get a warning:

/usr/local/lib/python2.7/dist-packages/pybindgen/cppclass_typehandlers.py:1120: UserWarning: Returning shared pointers is dangerous! The C++ API should be redesigned to avoid this situation.
  warnings.warn("Returning shared pointers is dangerous!"

For now, the functionality works, so we might go with it, but with a health warning. The returned list is to be used on the spot, or should be copied to a native list for safety. When I get time I'd like to extend the functionality of ReturnValue (I believe that this is the right place to affect getters and methods that return lists) to depend not on the container being supported, but to translate to a native container instead... That's for the future, as the code works now, with a small health warning.

If you have any other ideas though, I'm all ears and appreciative :-).

I'll mark this as problem solved for now and start a new discussion if I have further issues.

Many thanks for your help.