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 609866 - gvfs-gdu-volume-monitor crashed with SIGSEGV in gdu_pool_get_presentables()
gvfs-gdu-volume-monitor crashed with SIGSEGV in gdu_pool_get_presentables()
Status: RESOLVED INVALID
Product: gnome-disk-utility
Classification: Core
Component: libgdu
2.28.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-disk-utility-maint
Depends on:
Blocks:
 
 
Reported: 2010-02-13 20:02 UTC by Mario Limonciello
Modified: 2010-02-15 16:46 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
patch to fix the crash (572 bytes, patch)
2010-02-13 20:02 UTC, Mario Limonciello 		none Details | Review

Description Mario Limonciello 2010-02-13 20:02:09 UTC 
Created attachment 153727 [details] [review]
patch to fix the crash

This bug is being forwarded up from Ubuntu Lucid in this bug: https://bugs.edge.launchpad.net/ubuntu/+source/gnome-disk-utility/+bug/521481

When gdu_pool_get_presentables is called with a NULL pointer, it causes a sigsegv.  The attached patch fixes this.
Comment 1 Martin Pitt 2010-02-14 10:59:25 UTC 
Hm, this seems to only cure a symptom, not the actual cause? In the interest of robustness and debugging I agree that it'd be nice to add a g_return_val_if_fail(pool != NULL, NULL) to the function (which is better than just silently returning), but I think it's an error to call the function with NULL in the first place, since that just hides logic errors.
Comment 2 David Zeuthen (not reading bugmail) 2010-02-15 15:49:23 UTC 
Right, the patch doesn't really fix anything - it just masks the problem.

The only way we can get into this situation (GduPool object construction failing) is if the app can't talk to the udisks service. Since the udisks service is activated on demand, it means this can only fail if a) udisks fails to start; and/or b) dbus activation is buggy. It turned out that dbus activation was more or less buggy, see recent D-Bus commits which should fix these issues

Additionally, we now complain on stderr, see e.g.

http://git.gnome.org/browse/gnome-disk-utility/tree/src/gdu/gdu-pool.c?id=aa729095f188ad8dbe036af375e1da6f32590470#n1942

if we for some reason can't construct a GduPool object.

I still think the right answer is to crash if people try to use the return value from gdu_pool_new() and it's NULL... this is so we get the bug report (triggered by e.g. abrt) when this happens. In fact, it is partly because of these bug reports that we finally fixed the D-Bus activation bugs.
Comment 3 Martin Pitt 2010-02-15 15:52:50 UTC 
David,

I'd still like to add some g_return_val_if_fail() bits to make this easier to debug in the future, is that okay with you?
Comment 4 David Zeuthen (not reading bugmail) 2010-02-15 16:23:54 UTC 
(In reply to comment #3)
> David,
> 
> I'd still like to add some g_return_val_if_fail() bits to make this easier to
> debug in the future, is that okay with you?

Yup, sounds good. Thanks.
Comment 5 Martin Pitt 2010-02-15 16:40:06 UTC 
Sorry, of course I mean g_assert(), since we do want to crash the program (in order to not paper over the problem).