After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 574306 - gperl_callback_invoke() reference leak
gperl_callback_invoke() reference leak
Status: RESOLVED FIXED
Product: gnome-perl
Classification: Bindings
Component: Glib
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk2-perl-bugs
gtk2-perl-bugs
Depends on:
Blocks:
 
 
Reported: 2009-03-05 21:10 UTC by Kevin Ryde
Modified: 2009-06-06 17:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
failing .t case (1.09 KB, patch)
2009-03-05 21:12 UTC, Kevin Ryde
committed Details | Review
patch for gperl_callback_invoke() (1.56 KB, patch)
2009-03-05 21:14 UTC, Kevin Ryde
committed Details | Review

Description Kevin Ryde 2009-03-05 21:10:15 UTC
With the current cvs and debian i386 perl 5.10.0, in the additions to GtkHBox.t below the last test case fails.

The two new tests ask that a label widget is destroyed when weakened, after it's been added to and removed from the box.  The first test succeeds, but the second test, where the label has also been subject to a $box->foreach, fails.

Some digging suggests gperl_callback_invoke() is incrementing the C-side ref_count, ie. g_object_ref(), and never unreffing.  I believe it may be due to not having g_value_unset(), per change below.

I don't think anywhere in Glib-Perl itself puts objects through GPerlCallback, to exercise the problem, hence a test case only for gtk.  I noticed it with GtkTreeModelFilter when calling out to a "modify" function, but gtk_container_foreach() is a much simpler failing example.
Comment 1 Kevin Ryde 2009-03-05 21:12:55 UTC
Created attachment 130154 [details] [review]
failing .t case
Comment 2 Kevin Ryde 2009-03-05 21:14:32 UTC
Created attachment 130155 [details] [review]
patch for gperl_callback_invoke()
Comment 3 Torsten Schoenfeld 2009-03-08 18:45:20 UTC
Wow, that's a gigantic leak.  Patch committed, thanks!
Comment 4 Kevin Ryde 2009-03-10 00:26:59 UTC
I had moved the GValue variable out of the loop since g_value_unset() clears it to zeros, no need for a fresh initializer to do that ... which may save 2 nanoseconds.
Comment 5 Torsten Schoenfeld 2009-03-28 10:46:06 UTC
I didn't apply that part of the patch because it didn't seem related to the leak.

Do you think it's worth applying the change now, given the decrease in readability it entails?   (The variable is declared farther from where it's used, and it's not obvious that g_value_unset zero-fills the value, even though it's documented.)
Comment 6 Kevin Ryde 2009-03-29 23:55:56 UTC
So much of glib/gtk is poorly documented it makes you want to use the things that are documented! :-)  I get the impression set/unset/set/unset is the intended pattern for a given GValue location.
Comment 7 Torsten Schoenfeld 2009-06-06 17:44:23 UTC
OK, I just committed the optimization part of your original patch.