GNOME Bugzilla – Bug 672917
gnome-settings-daemon crashes if stylus id not found in libwacom database
Last modified: 2012-09-12 12:56:38 UTC
Created attachment 210698 [details] backtrace This is a follow-up on bug #672449 The fix for bug #672449 is not sufficient as an incomplete device definition will cause a crash in gsd-wacom-device.c as well: wacom-plugin:ERROR:gsd-wacom-device.c:1491:gsd_wacom_device_set_current_stylus: assertion failed: (device->priv->styli) To reproduce, I modified the definition of the device to include just one Stylus 0xfffff as follow: Styli=0xffffff And I get the same backtrace: (gdb) bt
+ Trace 229961
$1 = 0x0 (gdb) p *device->priv $2 = {gdk_device = 0x65d620, device_id = 14, opcode = 141, type = WACOM_TYPE_ERASER, name = 0x990750 "Bamboo One", path = 0x8832d0 "/dev/input/event10", icon_name = 0x7fffe9b48020 "wacom-tablet", tool_name = 0x9908d0 "Wacom Bamboo1 eraser", reversible = 1, is_screen_tablet = 0, styli = 0x0, last_stylus = 0x0, buttons = 0x0, modes = 0x0, num_modes = 0x0, wacom_settings = 0x7fffd8003400} So I think the assert() in gsd_wacom_device_set_current_stylus() can be reached because add_stylus_to_device() which is called from the cnstructor won't create the list if libwacom_stylus_get_for_id() fails and return NULL: 822 static void 823 add_stylus_to_device (GsdWacomDevice *device, 824 const char *settings_path, 825 int id) 826 { 827 const WacomStylus *wstylus; 828 829 wstylus = libwacom_stylus_get_for_id (db, id); 830 if (wstylus) { 831 GsdWacomStylus *stylus; ... 850 g_free (stylus_settings_path); 851 device->priv->styli = g_list_prepend (device->priv->styli, stylus); 852 } 853 } So if the device definition is incomplete, libwacom_stylus_get_for_id() return NULL, device->priv->styli is not modified and remains NULL, thus triggering the asser later down the code in gsd_wacom_device_set_current_stylus()
Created attachment 210699 [details] [review] Possible fix This patch removes the assert and issue a warning instead. Tried o na local reproducer and g-s-d does not crash anymore even with an incomplete stylus definition in libwacom. Note, gnome-control-center has a copy of gsd-wacom-device.c and will require a similar fix as well.
That needs to be fixed in libwacom, and the tests/load.c test needs to fail for incomplete definitions. I would instead ignore the definition if incomplete (in libwacom), and return a generic tablet instead.
But how does libwacom can tell a definition is incomplete?
Check whether there are missing fields in libwacom_parse_tablet_keyfile().
But the field is not missing, it's there but does not list all styli... e.g. "Styli=0xfffff" alone in the device definition of the "Bamboo Pen & Touch" tablet will cause a crash in g-s-d and g-c-c, whereas "Styli=0xfffff;0xffffe" wont. That's from downstream bug https://bugzilla.redhat.com/show_bug.cgi?id=799667
With the newest libwacom 0.5 this is also affecting other devices such as the Bamboo One. e.g. the new libwacom's tools/libwacom-list-local-devices gives: [Device] Name=Bamboo One DeviceMatch=usb:056a:0069; Class=Bamboo Width=5 Height=4 Styli=0xfffff; [Features] Reversible=true Stylus=true Ring=false Ring2=false BuiltIn=false Touch=false NumStrips=0 Buttons=0 --------------------------------------------------------------- And that "Styli" definition crashes g-s-d and control-center wacom panel.
This is because of this commit: http://linuxwacom.git.sourceforge.net/git/gitweb.cgi?p=linuxwacom/libwacom;a=commitdiff;h=1680f83acda9d6f191aa7b013e4621a44325cdcf With libwacom 0.5 if a tablet definition has no stylus defined, "Styli=0xfffff" is assumed and that's precisely what breaks g-s-d and control-center wacom panel (comment #0)
(In reply to comment #7) > With libwacom 0.5 if a tablet definition has no stylus defined, "Styli=0xfffff" > is assumed and that's precisely what breaks g-s-d and control-center wacom > panel (comment #0) The problem with libwacom-0.5 is now fixed in libwacom with the fix for bug 675299. Yet the problem of g-s-d/control-center crashing with incomplete stylus definitions still remains with some device, see downstream bug [1] [1] https://bugzilla.redhat.com/show_bug.cgi?id=799667
Created attachment 213433 [details] [review] 0001-wacom-if-no-stylis-were-added-for-this-device-use-th.patch add_stylus_for_device bails out if the styli doesn't match the device, so the generic 0xfffff stylus isn't added to the eraser tool. We end up with zero styli on the eraser. One stylus is defined in the data file though, so we end up with a NULL stylus list, causing asserts later in set_current_stylus. this patch simply changes the fallback approach from "are there any stylis defined in the database" to "did we add any stylis to the device".
(In reply to comment #8) > Yet the problem of g-s-d/control-center crashing with incomplete stylus > definitions still remains with some device, see downstream bug [1] I really don't want to have to think that there might be bad data in libwacom. libwacom shouldn't ship bad data instead. Added code to the tablet verification tests to check that for each stylus that is said to have an eraser, we have the matching eraser listed in the tablet's stylus list. This should fix this bug. commit e65080cd10ee9e5fca50d517bad2d1dd877d2b78 Author: Bastien Nocera <hadess@hadess.net> Date: Thu May 17 16:38:58 2012 +0100 data: Add missing types to some eraser IDs commit 8227f3f62fd015131d42efa97d0a6f19745bb968 Author: Bastien Nocera <hadess@hadess.net> Date: Thu May 17 16:36:48 2012 +0100 data: Add FIXME for DTI520UB/L's stylus Does its stylus have an eraser? If so, the default stylus will do, if not, we'll need to declare another generic stylus. commit ae4eaaa0bf4762c16f17ad9a5af66c60fc5b9065 Author: Bastien Nocera <hadess@hadess.net> Date: Thu May 17 16:35:38 2012 +0100 data: Bamboo Pen&touch's stylus has 2 buttons and an eraser Just like the default stylus. You're not that special. commit 4a7a7f16564869b3af6701aa943e5c88927f451b Author: Bastien Nocera <hadess@hadess.net> Date: Thu May 17 16:34:01 2012 +0100 test: Test each stylus with an eraser Each stylus declared to have an eraser should have the eraser defined in its stylus list as well. See https://bugzilla.gnome.org/show_bug.cgi?id=672917 > [1] https://bugzilla.redhat.com/show_bug.cgi?id=799667
The g-s-d code is still fundamentally buggy though. add_stylus_to_device() may still exist early and not add the stylus, yet in the caller you essentially rely on the styli being added. IMO you should check if there any styli were actually added, instead of assuming they were.
(In reply to comment #11) > The g-s-d code is still fundamentally buggy though. add_stylus_to_device() may > still exist early and not add the stylus, yet in the caller you essentially > rely on the styli being added. IMO you should check if there any styli were > actually added, instead of assuming they were. Yep, let's assert that they were added. The only cases where we could crash now are if libwacom gives us broken data, or when we get that elusive stylus with no eraser defined. When we get to that, we should make the eraser GsdWacomDevice just not instantiate. commit 62f51ac11a4ec764b6029cb81c7f82a007845368 Author: Bastien Nocera <hadess@hadess.net> Date: Fri May 18 10:23:39 2012 +0100 wacom: Don't add fallback styli ourselves The libwacom code should do that for us. https://bugzilla.gnome.org/show_bug.cgi?id=672917
*** Bug 670461 has been marked as a duplicate of this bug. ***
*** Bug 677643 has been marked as a duplicate of this bug. ***
It still happen with all gnome 3.x, even with 3.5.91 (using libwacom-0.5). Gnome classic either crashes when plugging the tablet or just after log in in GDM if the tablet is plugged before: (gnome-settings-daemon:15012): wacom-plugin-WARNING **: Could not set the current stylus ID 0x0 for tablet 'Wacom Graphire', no general pen found ** wacom-plugin:ERROR:gsd-wacom-device.c:1618:gsd_wacom_device_set_current_stylus: assertion failed: (device->priv->styli) gnome-session[14969]: WARNING: Application 'gnome-settings-daemon.desktop' killed by signal 6 Fontconfig warning: "/etc/fonts/conf.d/50-user.conf", line 9: reading configurations from ~/.fonts.conf is deprecated. (gnome-settings-daemon:15037): power-plugin-WARNING **: gnome-session is not available (...) (gnome-settings-daemon:15037): wacom-plugin-WARNING **: Could not set the current stylus ID 0x0 for tablet 'Wacom Graphire', no general pen found ** wacom-plugin:ERROR:gsd-wacom-device.c:1618:gsd_wacom_device_set_current_stylus: assertion failed: (device->priv->styli) gnome-session[14969]: WARNING: Application 'gnome-settings-daemon.desktop' killed by signal 6 gnome-session[14969]: WARNING: App 'gnome-settings-daemon.desktop' respawning too quickly (...) gnome-session[14969]: CRITICAL: gsm_manager_set_phase: assertion `GSM_IS_MANAGER (manager)' failed gnome-session[14969]: Gtk-CRITICAL: gtk_main_quit: assertion `main_loops != NULL' failed
Thierry, it sounds like libwacom is broken. File a separate bug about your problem, and it'll be investigated. It's not because the results are the same that the cause are the same.
Well we did open a bug report and it got tagged as a duplicate of this one (similar backtrace, same assert). Now it works OK with this september's libwacom-0.6 (actually I upgrade the xorg wacom driver at the same time but the odds're high it's not it which causes the issue). Whereas or not it's libwacom that breaks gnome, it's easy enough to check for libwacom version either at build time or run time and disable the plugin. It looks like libwacom < 0.6 and gnome-settings-daemon do not like each other so it would be better to disable the wacom plugin. You've enough reports showing this is affecting people...
Created attachment 224069 [details] [review] Proposed patch Now that libwacom 0.6 is released, upgrade libwacom required version to 0.6 to avoid hitting this assertion in gnome-settings-daemon
Review of attachment 224069 [details] [review]: Pushed.
*** Bug 683826 has been marked as a duplicate of this bug. ***