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 735486 - overriding GtkOverlay's get-child-position appears to not be setting the child allocation
overriding GtkOverlay's get-child-position appears to not be setting the chil...
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: introspection
3.13.x
Other Linux
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
Depends on:
Blocks:
 
 
Reported: 2014-08-26 20:54 UTC by David Shea
Modified: 2014-08-29 21:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
overlay test (1.26 KB, text/plain)
2014-08-26 20:54 UTC, David Shea
  Details
Analysis script (1.12 KB, text/plain)
2014-08-29 21:25 UTC, Simon Feltman
  Details
Special case signal output arguments which are structs as pass-by-reference (5.10 KB, patch)
2014-08-29 21:41 UTC, Simon Feltman
committed Details | Review

Description David Shea 2014-08-26 20:54:44 UTC
Created attachment 284549 [details]
overlay test

Attached a small script that demonstrates the issue with gtk-3.13.7 as packaged for Fedora 21. If I comment out get-child-position connect line, everything works as expected: the overlaying GtkImage is allocated to a 300x300 rectangle and everything appears normally. If I attempt to override the allocation, the overlaying image gets a random width, like 26326512, and a height of 1.
Comment 1 David Shea 2014-08-27 22:02:52 UTC
This might be an issue with gobject-introspection? Similar code in C works just fine, but using the gobject-introspection interface the members of the allocation out parameter are never modified from the uninitialized values in the gtk_overlay_compute_child_allocation stack allocation.
Comment 2 Matthias Clasen 2014-08-28 18:30:50 UTC
The signal has an (out caller-allocates) annotation, which seems correct to me.

But yeah, looks like an introspection problem to me.
Comment 3 Christoph Reiter (lazka) 2014-08-28 18:51:19 UTC
Bisected pygobject with gtk+ 3.12.2

85175047e66dfc0c0263eac91d8056a95d0a60a0 is the first bad commit
commit 85175047e66dfc0c0263eac91d8056a95d0a60a0
Author: Simon Feltman <sfeltman@src.gnome.org>
Date:   Tue Jul 29 19:29:28 2014 -0700

    Refactor boxed wrapper memory management strategy
    
    Change pygi_boxed_new() to accept "copy_boxed" instead of "free_on_dealloc".
    This changes memory management so the PyGIBoxed wrapper owns the boxed
    pointer given to it. Use __del__ instead of dealloc for freeing the boxed
    memory. This is needed for edge cases where objects like GSource can
    trigger the finalized callback during de-alloc, resulting in the PyObjects
    references counts being manipulated and triggering a re-entrant de-alloc.
    Add hack to keep Gtk.TreeIter.do_iter_next/previous implementations working
    which rely on pass-by-reference.
    See also: https://bugzilla.gnome.org/show_bug.cgi?id=734465
    
    https://bugzilla.gnome.org/show_bug.cgi?id=722899
    
    https://bugzilla.gnome.org/show_bug.cgi?id=726999
Comment 4 Simon Feltman 2014-08-28 20:39:11 UTC
Thanks the bisect Christoph. This is fallout from fixing bug 722899 with the addition that output arguments to signals don't work in pygi (bug 644927). Out arguments should really be returned from the signal callback, but in this case the combination of memory mis-management of boxed types and signal output arguments being treated as input arguments, we hit a case where the arguments can actually be modified and seem to "work" in the sense that they are passed by reference to Python.

In any event, I'll do a little more analysis and either special case this event or change all boxed signal args to continue using pass-by-reference semantics (which will suffer potential memory errors noted in bug 722899).
Comment 5 Simon Feltman 2014-08-29 21:25:11 UTC
Created attachment 284858 [details]
Analysis script

Interestingly "Gtk.Overlay:get-child-position" is the only signal out of all the typelibs on my system which has a struct output argument.
Comment 6 Simon Feltman 2014-08-29 21:41:30 UTC
The following fix has been pushed:
28d0337 Special case signal output arguments which are structs as pass-by-reference
Comment 7 Simon Feltman 2014-08-29 21:41:33 UTC
Created attachment 284860 [details] [review]
Special case signal output arguments which are structs as pass-by-reference

Add a special case which avoids copying of struct arguments marked as output
to signals. Since we don't currently support output arguments, users have
come to rely on a pass-by-reference bug which was fixed and caused this to
regress (bug 722899). Add unittest which is currently failing due to a number
of issues with emit() not supporting type annotations or output arguments
(bug 735693).