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 696707 - g-s-d cursor plugin doesn't fail gracefully
g-s-d cursor plugin doesn't fail gracefully
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
3.8.1
Depends on:
Blocks:
 
 
Reported: 2013-03-27 15:42 UTC by Emilio Pozuelo Monfort
Modified: 2013-04-15 11:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
cursor: add error quark (2.27 KB, patch)
2013-03-27 15:42 UTC, Emilio Pozuelo Monfort
rejected Details | Review
cursor: set an error if we can't create a monitor (1.22 KB, patch)
2013-03-27 15:42 UTC, Emilio Pozuelo Monfort
needs-work Details | Review
cursor: set error if we don't have the necessary X support (1.49 KB, patch)
2013-03-27 15:42 UTC, Emilio Pozuelo Monfort
needs-work Details | Review
cursor: set an error if we can't create a monitor (1.17 KB, patch)
2013-04-04 11:47 UTC, Emilio Pozuelo Monfort
committed Details | Review
cursor: set error if we don't have the necessary X support (1.39 KB, patch)
2013-04-04 11:47 UTC, Emilio Pozuelo Monfort
accepted-commit_now Details | Review

Description Emilio Pozuelo Monfort 2013-03-27 15:42:30 UTC
Hi,

Bug #696118 added code to gracefully fail if running on an
old xserver. Unfortunately the gnome-3-8 code doesn't pass
the GError to gnome-desktop, which means the GError is unset
when returning from gsd_cursor_manager_start() and we crash
when trying to print a g_warning() with error->message:

Program received signal SIGSEGV, Segmentation fault.
0xb36095de in impl_activate (plugin=0x80acb30) at gsd-cursor-plugin.c:29
29	GNOME_SETTINGS_PLUGIN_REGISTER (GsdCursor, gsd_cursor)
(gdb) bt
    at gnome-settings-plugin-info.c:431
    at gnome-settings-manager.c:95
    func=func@entry=0x804bfa0 <maybe_activate_plugin>, user_data=user_data@entry=0x0)
    at /tmp/buildd/glib2.0-2.36.0/./glib/gslist.c:896

Note that the same will happen if one of the X checks fail.

The attached patches fix this. They are against gnome-3-8, but I think we
should include the first and the third on master (the second is not needed
on master as gnome_idle_monitor_new_for_device() takes the GError there.
Comment 1 Emilio Pozuelo Monfort 2013-03-27 15:42:33 UTC
Created attachment 239960 [details] [review]
cursor: add error quark
Comment 2 Emilio Pozuelo Monfort 2013-03-27 15:42:37 UTC
Created attachment 239961 [details] [review]
cursor: set an error if we can't create a monitor

Otherwise we crash in impl_activate()
Comment 3 Emilio Pozuelo Monfort 2013-03-27 15:42:41 UTC
Created attachment 239962 [details] [review]
cursor: set error if we don't have the necessary X support

Otherwise we crash in impl_activate()
Comment 4 Antoine Jacoutot 2013-04-01 19:30:23 UTC
I can confirm this fixes an immediate runtime crash of gnome-settings-daemon on OpenBSD.
I'd really like this or a variation of it to be pushed...
Thanks.
Comment 5 Bastien Nocera 2013-04-02 08:21:45 UTC
Review of attachment 239960 [details] [review]:

No need for this, the error can be the same used in gnome-desktop master.
Comment 6 Bastien Nocera 2013-04-02 08:22:15 UTC
Review of attachment 239961 [details] [review]:

::: plugins/cursor/gsd-cursor-manager.c
@@ +180,3 @@
+        if (!monitor) {
+                g_set_error (error, GSD_CURSOR_MANAGER_ERROR,
+                             GSD_CURSOR_MANAGER_ERROR_FAILED,

Use G_IO_ERROR and G_IO_ERROR_NOT_SUPPORTED instead.
Comment 7 Bastien Nocera 2013-04-02 08:23:04 UTC
Review of attachment 239962 [details] [review]:

::: plugins/cursor/gsd-cursor-manager.c
@@ +296,2 @@
         if (supports_cursor_xfixes () == FALSE) {
+                g_set_error (error, GSD_CURSOR_MANAGER_ERROR,

Ditto.

This needs to go to master as well.
Comment 8 Emilio Pozuelo Monfort 2013-04-04 11:47:09 UTC
Created attachment 240587 [details] [review]
cursor: set an error if we can't create a monitor

Otherwise we crash in impl_activate()
Comment 9 Emilio Pozuelo Monfort 2013-04-04 11:47:13 UTC
Created attachment 240588 [details] [review]
cursor: set error if we don't have the necessary X support

Otherwise we crash in impl_activate()
Comment 10 Emilio Pozuelo Monfort 2013-04-13 09:45:24 UTC
OK to commit these two to gnome-3-8 and the last one to master? I'd like to get this in for 3.8.1.
Comment 11 Bastien Nocera 2013-04-15 09:01:22 UTC
Review of attachment 240587 [details] [review]:

::: plugins/cursor/gsd-cursor-manager.c
@@ +171,3 @@
+        if (!monitor) {
+                g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
+                             "Per-device idletime monitor not available");

Fine.
Comment 12 Bastien Nocera 2013-04-15 09:01:44 UTC
Review of attachment 240588 [details] [review]:

Looks good.
Comment 13 Emilio Pozuelo Monfort 2013-04-15 11:27:23 UTC
Comment on attachment 240587 [details] [review]
cursor: set an error if we can't create a monitor

Pushed to gnome-3-8
Comment 14 Emilio Pozuelo Monfort 2013-04-15 11:27:38 UTC
Comment on attachment 240588 [details] [review]
cursor: set error if we don't have the necessary X support

Pushed to gnome-3-8 and master.