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 670033 - gconf-dbus: don't crash during sync if gconfd shutting down
gconf-dbus: don't crash during sync if gconfd shutting down
Status: RESOLVED FIXED
Product: GConf
Classification: Deprecated
Component: gconf
unspecified
Other All
: Normal normal
: ---
Assigned To: GConf Maintainers
GConf Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-02-14 04:45 UTC by Ray Strode [halfline]
Modified: 2012-02-17 17:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gconf-dbus: don't crash during sync if gconfd shutting down (1.50 KB, patch)
2012-02-14 04:45 UTC, Ray Strode [halfline]
committed Details | Review

Description Ray Strode [halfline] 2012-02-14 04:45:16 UTC
I was asked to look at this downstream bug:

https://bugzilla.redhat.com/show_bug.cgi?id=756245

which seems to be affecting a number of F16 users.

Looking at the backtraces and logs on that bug along with the
source code, it appears to be a missing return statement
in gconf_engine_suggest_sync.
Comment 1 Ray Strode [halfline] 2012-02-14 04:45:18 UTC
Created attachment 207512 [details] [review]
gconf-dbus: don't crash during sync if gconfd shutting down

The gconfd shuts down after a bit of inactivity.  When that
happens there's a window where it refuses requests from the
client library with an error.  The library is resposible for
gracefully handling this condition and reacting appropriately.

There are many places in the code where the client library has
this idiom:

db = gconf_engine_get_database (conf, TRUE, err);

if (db == NULL)
  {
    g_return_if_fail(err == NULL || *err != NULL);

    return;
  }

In the event gconfd is shutting down, db will be NULL, and the
code will return early from whatever (non-critical) operation
it was doing.

gconf_engine_suggest_sync has a similiar chunk of code, but it
neglected the "return;" and then promptly crashed since it wasn't
expecting db to be NULL.

This commit adds the return;
Comment 2 Ray Strode [halfline] 2012-02-14 04:48:15 UTC
Note i haven't reproduced the problem myself, so I don't know for sure this fix is right.  I'll do a testing update on the downstream report and request feedback.
Comment 3 Ray Strode [halfline] 2012-02-16 19:54:48 UTC
Downstream seems to show this works fine.  Unless someone wants to review this (Ross? Rob?) I'll probably push it in the next few days.
Comment 4 Ross Burton 2012-02-17 09:09:10 UTC
Review of attachment 207512 [details] [review]:

Looks good please push.
Comment 5 Ross Burton 2012-02-17 09:09:11 UTC
Review of attachment 207512 [details] [review]:

Looks good please push.
Comment 6 Ross Burton 2012-02-17 09:09:11 UTC
Review of attachment 207512 [details] [review]:

Looks good please push.
Comment 7 Ray Strode [halfline] 2012-02-17 17:39:46 UTC
I didn't quite catch that, did you say it looks good? :-)
Comment 8 Ray Strode [halfline] 2012-02-17 17:41:14 UTC
Attachment 207512 [details] pushed as 6f3e127 - gconf-dbus: don't crash during sync if gconfd shutting down
Comment 9 Ross Burton 2012-02-17 17:41:52 UTC
Ha, what the hell happened there!