Re: Problems with un-owned objects passed to closures in pygobject (gtk_cell_renderer_text_start_editing)



After some deliberation and writing a bunch of tests: http://git.gnome.org/browse/pygobject/commit/?id=97f48f

I am starting to warm up to an idea where we simply never sink objects and always follow the rules entailed by ownership transference annotations already in place, with one caveat: g_object_new is annotated as transfer full but can also return floating references. In this case, we must check the returned type and not believe the annotation when it returns InitiallyUnowned instances, but instead treat it like transfer none and add a new ref.

The reason I think this will work is because libraries that make use of InitiallyUnowned are designed to deal with the floating refs already. Python is basically a thin wrapper used to tie objects together within these systems and never needs to maintain ownership beyond making sure the underlying object is kept alive while the wrapper is alive. From this, we can rely on the internals of the libraries to do the right thing and sink floating references where they normally would in C usage. If they don't, it is most likely a bug in C as well (by convention). This seems like it will solve all of the current problems and special casing during marshaling. However, it also adds a leak for the most basic (and useless) case:
    for i in range(10):
        Gtk.Button()

This would leak the initial floating ref and the memory would be lost. However, I can't think of a real use case where something like that would ever be needed.

The alternatives to can become grossly convoluted:
https://bugzilla.gnome.org/show_bug.cgi?id=687522#c15

Thoughts?

-Simon


 


On Tue, Jan 29, 2013 at 3:44 AM, Simon Feltman <s feltman gmail com> wrote:
I tend to agree we should be avoiding reliance on main loops (or GC timing) to get the ref counts right if possible.

PyGObject also uses toggle refs similarly to gjs for keeping the wrappers alive. However, in PyGObject this only happens if a Python instance attribute is set. Whereas with gjs it seems to use a toggle ref all the time just in case an attribute is set?

It seems like the problem at hand can be solved by maintaining the floating ref and adding our own safety ref for the wrapper. With one caveat: upon completion of the python callback we may consider sinking the GObject if the ref is floating and the Python wrapper has a reference count greater than one. This basically means code in the callback made an assignment of the object to something outside of its scope and that should be considered a strong reference. But that might not even be necessary. I've attempted to describe this along with all the other problematic reference counting situations in a separate document:

The biggest concern at this point is how to properly deal with vfunc implementations which return objects and are annotated as "transfer none". Review, corrections, and feedback is very welcome.

Thanks,
-Simon



On Fri, Jan 18, 2013 at 12:19 AM, Tristan Van Berkom <tvb gnome org> wrote:
On Fri, Jan 18, 2013 at 5:49 AM, Giovanni Campagna
<scampa giovanni gmail com> wrote:
[...]
> I know that Python doesn't have a GC in the traditional sense, but you
> could still send finalization for GObject wrappers to a idle callback
> so there is no risk of finalizing objects that C code assumes are
> still alive.

That doesn't sound like a very safe workaround to me.

There are situations where a lot of code can run without the mainloop
ever becoming idle, while running a ClutterTimeline is one of those
cases (or at least I've observed that idle callbacks dont generally
get called while a ClutterTimeline is playing, perhaps they do with
an ultra high priority).

Another thing to consider is that not all code written with the glib
stack is actually reactive & event based, code that does not run
a mainloop will risk blowing up in size quickly, possibly attaining
out of memory conditions unnecessarily if the code happens to
be highly recursive.

Cheers,
                -Tristan




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]