[prev in list] [next in list] [prev in thread] [next in thread]
List: gtk-devel
Subject: Re: Problems with un-owned objects passed to closures in pygobject (gtk_cell_renderer_text_start_edi
From: Simon Feltman <s.feltman () gmail ! com>
Date: 2013-02-04 2:39:20
Message-ID: CACc9j8YYV2ycNKXTpQqDCqsgxXTiNmYKP2F5Bnx1hKDjsrMgWA () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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:
>
> https://live.gnome.org/PyGObject/Analysis/ObjectReferenceCountingForVFuncsAndClosures
>
> 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
>>
>
>
[Attachment #5 (text/html)]
<div dir="ltr">After some deliberation and writing a bunch of tests: <a \
href="http://git.gnome.org/browse/pygobject/commit/?id=97f48f">http://git.gnome.org/browse/pygobject/commit/?id=97f48f</a><div><br></div><div \
style>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.</div>
<div style><br></div><div style>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:</div>
<div style> for i in range(10):</div><div style> Gtk.Button()</div><div \
style><br></div><div style>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.</div>
<div style><br></div><div style>The alternatives to can become grossly \
convoluted:</div><div style><a \
href="https://bugzilla.gnome.org/show_bug.cgi?id=687522#c15">https://bugzilla.gnome.org/show_bug.cgi?id=687522#c15</a><br>
</div><div style><br></div><div style>Thoughts?</div><div style><br></div><div \
style>-Simon</div><div style><br></div><div style><br></div><div style> \
</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Tue, Jan 29, 2013 at 3:44 AM, Simon Feltman <span dir="ltr"><<a \
href="mailto:s.feltman@gmail.com" target="_blank">s.feltman@gmail.com</a>></span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex">
<div dir="ltr"><div>I tend to agree we should be avoiding reliance on main loops (or \
GC timing) to get the ref counts right if \
possible.<br></div><div><br></div><div>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?</div>
<div><br></div><div>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:</div>
<div><a href="https://live.gnome.org/PyGObject/Analysis/ObjectReferenceCountingForVFuncsAndClosures" \
target="_blank">https://live.gnome.org/PyGObject/Analysis/ObjectReferenceCountingForVFuncsAndClosures</a><br></div><div>
<br></div><div>
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.</div><div> \
<br></div><div>Thanks,</div><div>-Simon</div><div><br></div></div><div \
class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div \
class="gmail_quote">On Fri, Jan 18, 2013 at 12:19 AM, Tristan Van Berkom <span \
dir="ltr"><<a href="mailto:tvb@gnome.org" \
target="_blank">tvb@gnome.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">On Fri, Jan 18, 2013 at 5:49 AM, Giovanni Campagna<br> <<a \
href="mailto:scampa.giovanni@gmail.com" \
target="_blank">scampa.giovanni@gmail.com</a>> wrote:<br> [...]<br>
<div>> I know that Python doesn't have a GC in the traditional sense, but \
you<br> > could still send finalization for GObject wrappers to a idle \
callback<br> > so there is no risk of finalizing objects that C code assumes \
are<br> > still alive.<br>
<br>
</div>That doesn't sound like a very safe workaround to me.<br>
<br>
There are situations where a lot of code can run without the mainloop<br>
ever becoming idle, while running a ClutterTimeline is one of those<br>
cases (or at least I've observed that idle callbacks dont generally<br>
get called while a ClutterTimeline is playing, perhaps they do with<br>
an ultra high priority).<br>
<br>
Another thing to consider is that not all code written with the glib<br>
stack is actually reactive & event based, code that does not run<br>
a mainloop will risk blowing up in size quickly, possibly attaining<br>
out of memory conditions unnecessarily if the code happens to<br>
be highly recursive.<br>
<br>
Cheers,<br>
-Tristan<br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
_______________________________________________
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic