Bug 1420706 - DisplayChannel monitors property is not usable in python
Summary: DisplayChannel monitors property is not usable in python
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: pygobject3
Version: 7.4
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: rc
: ---
Assignee: Matthew Barnes
QA Contact: Desktop QE
URL:
Whiteboard:
Depends On:
Blocks: 1339289
TreeView+ depends on / blocked
 
Reported: 2017-02-09 10:56 UTC by Pavel Hrdina
Modified: 2023-01-20 23:56 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-10-25 14:59:51 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Pavel Hrdina 2017-02-09 10:56:45 UTC
The "monitors" property is implemented as G_TYPE_ARRAY which is internally implemented as GArray of SpiceDisplayMonitorConfig.  Because the "monitors" property is defined as generic G_TYPE_ARRAY the pygobject doesn't know how to convert it to python and any usage of "monitors" property in python code prints this error message:

  CRITICAL **: Stack overflow protection. Can't copy array element into GIArgument.

The python code simply tries to get the property:

  ...
  monitors = displayChannel.get_property("monitors").
  ...


This blocks Bug 1339289 that requests to implement multi-monitor support into virt-maanger.

To solve this issue spice-gtk needs to add for example a new property that would be possible to use in python or introduce getter method/function that would return DisplayMonitorConfig for specific monitor based on index.

Comment 2 Christophe Fergeau 2017-02-09 13:56:08 UTC
Wouldn't it be enough to add an annotation to specify the type contained in the GArray ?

diff --git a/src/channel-display.c b/src/channel-display.c
index ca56cd1..0d025d0 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -284,7 +284,7 @@ static void spice_display_channel_class_init(SpiceDisplayChannelClass *klass)
                            G_PARAM_STATIC_STRINGS));

     /**
-     * SpiceDisplayChannel:monitors:
+     * SpiceDisplayChannel:monitors: (type GArray(SpiceDisplayMonitorConfig))
      *
      * Current monitors configuration.
      *


I can provide a scratch build if needed

Comment 3 Pavel Hrdina 2017-02-09 14:19:21 UTC
I've just tested it and it's not enough.  The thing is that the property is defined as G_TYPE_ARRAY and that's it.  Pygobject doesn't know how to what the type of items inside the array is so it tries to use a generic type and fails.

For example, "gl-scanout" is represented as single custom type and it works, because that type is described and it's possible to map it to some python objects.

Another example is builtin G_TYPE_STRV which is an array of strings and it works as well.

It might by pygobject bug or just limitation of what it can handle.  If you use the property in C code like virt-viewer does, you can simply get a pointer and type cast it to correct one, but I'm afraid that this is not an option for pygobject.

Comment 4 Christophe Fergeau 2017-02-09 14:30:15 UTC
(In reply to Pavel Hrdina from comment #3)
> I've just tested it and it's not enough.  The thing is that the property is
> defined as G_TYPE_ARRAY and that's it.  Pygobject doesn't know how to what
> the type of items inside the array is so it tries to use a generic type and
> fails. 

No, the property is not just defined as G_TYPE_ARRAY with the annotation. Before the change, /usr/share/gir-1.0/SpiceClientGLib-2.0.gir only describes the "monitors" property as:
      <property name="monitors" version="0.13" transfer-ownership="none">
        <doc xml:space="preserve">Current monitors configuration.</doc>
        <array name="GLib.Array">
          <type name="gpointer" c:type="gpointer"/>
        </array>
      </property>
so I agree that it's not possible for python to know its content.

However, after regenerating SpiceCLientGlib-2.0.gir with the change I suggested, the .gir content becomes:
      <property name="monitors" version="0.13" transfer-ownership="none">
        <doc xml:space="preserve">Current monitors configuration.</doc>
        <array name="GLib.Array">
          <type name="DisplayMonitorConfig"/>
        </array>
      </property>
so this time the type of the elements in the array is known.
I would expect that this is enough information for the python gobject-introspection layer to do the right thing and let you access the array content.

Comment 5 Pavel Hrdina 2017-02-09 14:55:18 UTC
I've built spice-gtk with that patch and it's not enough to make it work in python.  Still the same error.

Comment 6 Pavel Grunt 2017-02-10 09:34:03 UTC
I agree with the comment 4, moving to pygobject for investigation.

I searched little bit and found https://mail.gnome.org/archives/python-hackers-list/2015-February/msg00000.html so we are not the only one having the issue

Comment 7 Christophe Fergeau 2017-02-10 09:41:33 UTC
(executive summary, pygobject currently does not support array of structs, just arrays of pointers to struct)


Note You need to log in before you can comment on or make changes to this bug.