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 696118 - Causes gnome-settings-daemon to crash when running with Xorg 1.12
Causes gnome-settings-daemon to crash when running with Xorg 1.12
Status: RESOLVED FIXED
Product: gnome-desktop
Classification: Core
Component: libgnome-desktop
3.7.x
Other Linux
: Normal normal
: ---
Assigned To: Desktop Maintainers
Desktop Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-03-19 08:12 UTC by Sjoerd Simons
Modified: 2013-03-26 07:02 UTC
See Also:
GNOME target: 3.8
GNOME version: ---


Attachments
idle-monitor: Make per-device monitor fallible (4.92 KB, patch)
2013-03-23 09:05 UTC, Bastien Nocera
committed Details | Review
cursor: Update for gnome-desktop API change (6.00 KB, patch)
2013-03-23 09:05 UTC, Bastien Nocera
committed Details | Review
idle-monitor: Make per-device monitor fallible (3.88 KB, patch)
2013-03-25 07:56 UTC, Bastien Nocera
committed Details | Review
cursor: Update for gnome-desktop API change (6.06 KB, patch)
2013-03-25 08:42 UTC, Bastien Nocera
committed Details | Review

Description Sjoerd Simons 2013-03-19 08:12:28 UTC
Xorg 1.12 didn't implement the IDLETIMER counters yet, so when running gnome-settings-daemon on such a system the cursor plugin triggers a crash when calling gnome_idle_monitor_add_user_active_watch


$ gnome-settings-daemon 
(gnome-settings-daemon:8889): GnomeDesktop-WARNING **: GnomeIdleMonitor: IDLETIME counter not found

(gnome-settings-daemon:8889): Gdk-ERROR **: The program 'gnome-settings-daemon' received an X Window System error.
This probably reflects a bug in the program.
The error was 'XSyncBadAlarm'.
  (Details: serial 216 error_code 155 request_code 144 minor_code 9)
  (Note to programmers: normally, X errors are reported asynchronously;
   that is, you will receive the error a while after causing it.
   To debug your program, run it with the GDK_SYNCHRONIZE environment
   variable to change this behavior. You can then get a meaningful
   backtrace from your debugger if you break on the gdk_x_error() function.)
[1]    8889 trace trap (core dumped)  gnome-settings-daemon
Comment 1 Sjoerd Simons 2013-03-19 08:20:49 UTC
(In reply to comment #0)
> Xorg 1.12 didn't implement the IDLETIMER counters yet, so when running
> gnome-settings-daemon on such a system the cursor plugin triggers a crash when
> calling gnome_idle_monitor_add_user_active_watch

small correction, didn't implement the per-device DEVICEIDLETIME counter
Comment 2 Emanuele Aina 2013-03-19 12:13:13 UTC
For reference, disabling the cursor plugin is a quick workaround for this issue:

$ gsettings set org.gnome.settings-daemon.plugins.cursor active false
Comment 3 Matthias Clasen 2013-03-20 14:51:01 UTC
Can we have a workaround for that, Bastien ?
Comment 4 Bastien Nocera 2013-03-23 08:12:27 UTC
I would need to break API/ABI to fix that, but it's straight forward.
Comment 5 Bastien Nocera 2013-03-23 09:05:22 UTC
Created attachment 239618 [details] [review]
idle-monitor: Make per-device monitor fallible
Comment 6 Bastien Nocera 2013-03-23 09:05:40 UTC
Created attachment 239619 [details] [review]
cursor: Update for gnome-desktop API change

Do error checking, on startup, for the creation of per-device
idletime monitors. Note that we don't do error checking later
in the run-time as this should be the only the only possible
failure, bar system problems.
Comment 7 Bastien Nocera 2013-03-23 09:12:56 UTC
Patches for gnome-desktop and gnome-settings-daemon attached, please test.

(I'll note that, if this is considered too invasive, distributors can patch out the functionality if they ship an older X.org. The code has been there for 5 months, yet nobody from those distributors tested it...).
Comment 8 Sjoerd Simons 2013-03-23 09:57:42 UTC
(In reply to comment #7)
> Patches for gnome-desktop and gnome-settings-daemon attached, please test.
> 
> (I'll note that, if this is considered too invasive, distributors can patch out
> the functionality if they ship an older X.org. The code has been there for 5
> months, yet nobody from those distributors tested it...).

Unfortunately not all distributors have the resources to follow gnome development for the full 6 months. With my distributor hat on, if your patches don't make it for 3.8 we'd probably do another work-around until we move to newer Xorg (which should hopefully be soon).. Shipping a distro patch that changes ABI/API can't really be undone, nor dropped when it's no longer necessary making it quite awkward.

Personally i'd be very happy for Gnome to simply state it needs Xorg 1.14 as apart from this issue current mutter also triggers bugs in older X servers (fixed in X http://cgit.freedesktop.org/xorg/xserver/commit/?id=314776eb369ca2e438907795ae030dd743c281fc for x 1.13/1.14)
Comment 9 Matthias Clasen 2013-03-24 14:13:49 UTC
I don't see why we have to change the API here - you can just as well return NULL without setting an error. You still need gnome-desktop to avoid inserting NULL in the hash table, of course.
Comment 10 Bastien Nocera 2013-03-25 07:56:02 UTC
Comment on attachment 239618 [details] [review]
idle-monitor: Make per-device monitor fallible

Committed the patch with the API change to master.

Attachment 239618 [details] pushed as 5ca4cfc - idle-monitor: Make per-device monitor fallible
Comment 11 Bastien Nocera 2013-03-25 07:56:32 UTC
Created attachment 239732 [details] [review]
idle-monitor: Make per-device monitor fallible
Comment 12 Bastien Nocera 2013-03-25 08:41:05 UTC
Comment on attachment 239619 [details] [review]
cursor: Update for gnome-desktop API change

Pushed to g-s-d master.

Attachment 239619 [details] pushed as b3c1ee4 - cursor: Update for gnome-desktop API change
Comment 13 Bastien Nocera 2013-03-25 08:42:00 UTC
Created attachment 239734 [details] [review]
cursor: Update for gnome-desktop API change

gnome_idle_monitor_new_for_device() can now return NULL on
errors, such as when per-device idletime counters aren't available.

Do error checking, on startup, for the creation of per-device
idletime monitors. Note that we don't do error checking later
in the run-time as this should be the only the only possible
failure, bar system problems.
Comment 14 Bastien Nocera 2013-03-25 08:56:11 UTC
The 2 patches for gnome-3-8 attached. Can you please test them?
Comment 15 Sjoerd Simons 2013-03-25 12:10:36 UTC
Tested with ff962e302cea8a47ac6243e384806b04c5ffc103 (gnome-dekstop) and f63ee80368edea33500b4139b1c1289a7bfb29ff (gnome-settings-daemon) on top of their respective 3.8 branches seems to solve the issues and instead of a crash (correctly) prints:

 (lt-gsd-test-cursor:27686): cursor-plugin-DEBUG: Per-device idletime monitor not available, will not hide the cursor
Comment 16 Bastien Nocera 2013-03-26 06:58:11 UTC
Comment on attachment 239732 [details] [review]
idle-monitor: Make per-device monitor fallible

Attachment 239732 [details] pushed as 72a2bcc - idle-monitor: Make per-device monitor fallible
Comment 17 Bastien Nocera 2013-03-26 07:02:04 UTC
Pushed to gnome-3-8 branches too.

Attachment 239734 [details] pushed as 791338b - cursor: Update for gnome-desktop API change