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 672449 - wacom plugin can crash g-s-d if last-stylus property isn't previously set.
wacom plugin can crash g-s-d if last-stylus property isn't previously set.
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: wacom
3.3.x
Other Linux
: Normal major
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2012-03-20 09:45 UTC by Olivier Fourdan
Modified: 2012-03-23 11:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
backtrace (3.56 KB, text/plain)
2012-03-20 09:47 UTC, Olivier Fourdan
  Details
xsession errors (relevant part only) (1.08 KB, text/plain)
2012-03-20 09:47 UTC, Olivier Fourdan
  Details
Proposed patch (1014 bytes, patch)
2012-03-20 09:48 UTC, Olivier Fourdan
committed Details | Review

Description Olivier Fourdan 2012-03-20 09:45:27 UTC
The crash is caused by a call to g_error() that triggers an abort. 

The error is triggered because the plugin fails to retrieve the settings for the stylus, because the stylus is not set (stylus = 0x0, stylus_settings = 0x0 frame #3 of thread #1 in the backtrace above).

The stylus is obtained via the "last-stylus" property of the device, if the property is not set the value is 0x0 and the associated settings can't be retrieved either.

Same is clearly seen from the logs (provided xsession-errors log file):

  wacom-plugin-WARNING **: Failed to get value for changed stylus ID on device '11
  wacom-plugin-CRITICAL **: gsd_wacom_stylus_get_name: assertion `GSD_IS_WACOM_STYLUS (stylus)' failed
  wacom-plugin-CRITICAL **: gsd_wacom_stylus_get_settings: assertion `GSD_IS_WACOM_STYLUS (stylus)' failed
  GLib-GIO-CRITICAL **: g_settings_get_value: assertion `G_IS_SETTINGS (settings)' failed
  GLib-CRITICAL **: g_variant_get_type: assertion `value != NULL' failed
  GLib-CRITICAL **: g_variant_type_is_subtype_of: assertion `g_variant_type_check (type)' failed
  GLib-CRITICAL **: g_variant_get_fixed_array: assertion `g_variant_is_of_type (value, G_VARIANT_TYPE_ARRAY)' failed
  GLib-CRITICAL **: g_variant_unref: assertion `value != NULL' failed
  wacom-plugin-ERROR **: Pressurecurve requires 4 values.
  gnome-session[1027]: WARNING: Application 'gnome-settings-daemon.desktop' killed by signal
  
The "last-stylus" property is not retrieved because it's not been previously set, as shows the error "Failed to get value for changed stylus ID" earlier in the logs (that warning will cause the function to return before settings the property in filter_events() from gsd-wacom-device.c).

So I think the best way to avoid such a crash is to actually check for the property in apply_stylus_settings() so that if the property has not been set we do not end up trying to retreive the settings of an undefined stylus which will rigthfully lead to an error.

downstream bug: https://bugzilla.redhat.com/show_bug.cgi?id=799667

Proposed patch attached.
Comment 1 Olivier Fourdan 2012-03-20 09:47:14 UTC
Created attachment 210153 [details]
backtrace
Comment 2 Olivier Fourdan 2012-03-20 09:47:51 UTC
Created attachment 210154 [details]
xsession errors (relevant part only)
Comment 3 Olivier Fourdan 2012-03-20 09:48:40 UTC
Created attachment 210155 [details] [review]
Proposed patch
Comment 4 Olivier Fourdan 2012-03-20 09:53:26 UTC
Note: This patch is for gnome-settings-daemon where the affected source file belongs, but this very same file is duplicated in gnome-control-center when "make update-from-gsd" is run from within gnome-control-center/panels/wacom/

So this needs to be fixed in both gnome-settings-daemon and gnome-control-center packages.
Comment 5 Richard Hughes 2012-03-20 09:57:06 UTC
Review of attachment 210155 [details] [review]:

Looks good, thanks.
Comment 6 Olivier Fourdan 2012-03-23 09:30:59 UTC
Peter Hutterer found the root cause, it's in the definition of the "Styli" of the bamboo pen and touch tablet in libwacom, see [1].

The proposed fix still stands as it'd be safer if gnome-settings-daemon would not crash even if the definition of the tablet is incomplete.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=799667#c17
Comment 7 Richard Hughes 2012-03-23 10:28:18 UTC
Oliver, do we still need the g-c-c fix too? The original bug touched only gsd-wacom-manager.c which obviously does not exist in g-c-c.
Comment 8 Olivier Fourdan 2012-03-23 10:31:17 UTC
(In reply to comment #7)
> Oliver, do we still need the g-c-c fix too? The original bug touched only
> gsd-wacom-manager.c which obviously does not exist in g-c-c.

You're right, we don't.
Comment 9 Richard Hughes 2012-03-23 11:54:16 UTC
Cool, thanks for checking.