GNOME Bugzilla – Bug 700233
Gtk+ exits on X11 when querying a slave device
Last modified: 2013-05-20 01:22:44 UTC
I'm having a problem with rhythmbox exiting when clicking on the playlist expander on the left of the main window (that collapse/uncollapse the "My Top Rated", "Recently Added", "Recently Played" items). It turns out the widget controlling this expander tries to query the position of the device (bit weird to me, it could use the event instead...). Anyway, GTK+ shouldn't exit like this. Here is the backtrace :
+ Trace 231935
Created attachment 244062 [details] [review] x11: make _gdk_device_query_state report the master's device state I've seen other part of the API that just prevent querying anything on slave device like gdk_device_get_position/gdk_device_get_window_at_position but the query state is still accessible through the GdkWindow API. Maybe with this new approach we could relax the checks in GdkDevice.
whats the X error ? I don't think XIQueryPointer is supposed to give an error for slave devices.
We get an "X Error: BadDevice, invalid or uninitialized input device". There is a similar check in gdk_x11_device_xi2_set_window_cursor for example.
Xserver does return BadDevice for that call on slave devices that are attached to a master, as the position is then dependent on the master pointer one, and the button/modifier state can be just a subset of that one too, so that call is only meant to work on master and floating devices. http://cgit.freedesktop.org/xorg/xserver/tree/Xi/xiquerypointer.c#n104 Although from a look at the rhythmbox code, I see it's not using the source device from the event to query the position, but the device that ought typically to be a master device: https://git.gnome.org/browse/rhythmbox/tree/widgets/gossip-cell-renderer-expander.c#n326 So are events from slave devices somehow being enabled in rhythmbox? what for? I think the real error is behind that. As for the patch, I see it worthwhile to have GTK+ fall in less X errors, the right condition though is == GDK_DEVICE_TYPE_SLAVE , the patch as is turns this gdk call into a no-op for floating devices (these never have an associated device)
How can you enable slave devices?
either calling gdk_window_set_device_events() for a slave device, or more indirectly by doing gdk_device_grab() on a slave device, so it gets temporarily detached from the master and sends events on its own.
I reproduced that crash without the application ever calling either of these 2 functions...
Ok... after peeking a bit on annotate: commit 7293b612f96d03f8cc08741bd669ccf21c19b214 Author: Jonathan Matthew <jonathan@d14n.org> Date: Sat Apr 13 14:59:34 2013 +1000 use gdk_event_get_device instead of gdk_event_get_source_device We don't actually care about physical devices vs virtual devices, we just want to know where the pointer was. Now that we're using XI2 (see commit f84a33d), requesting the physical device position causes XI_BadDevice errors. https://bugzilla.gnome.org/show_bug.cgi?id=697915 So fixed in rhythmbox master, the GTK+ patch could still be fine with the change I proposed on comment #4
Thank you Carlos. Attaching the new patch. I'm also wondering about a couple of things. Currently the problem we encounter happens on a gdk_window_get_device_position() call. What about the gdk_device_get_position()/gdk_device_get_state() that ultimately end up calling query_state() on the xi2 backend too? Right now the generic code in gdkdevice.c prints warnings if you call these functions on a slave device. Should it be changed? GtkTreeView also calls gdk_window_get_device_position() using gdk_event_get_device(). Should that be changed in the same way rhythmbox changed its internal widgets?
Created attachment 244519 [details] [review] x11: make _gdk_device_query_state report the master's device state
Created attachment 244580 [details] [review] x11: make _gdk_device_query_state report the master's device state My mistake, wrong condition...
Review of attachment 244580 [details] [review]: ok
Attachment 244580 [details] pushed as ea6ac66 - x11: make _gdk_device_query_state report the master's device state