GNOME Bugzilla – Bug 777737
g_file_monitor_directory returns NULL and doesn't provide error
Last modified: 2017-11-10 13:30:50 UTC
It seems that g_file_monitor_directory may return NULL and doesn't provide any error in some cases. It should not happen and I don't really have any idea why it happens. As a consequence, network backend sometimes crashes probably because of dereferencing NULL error variable, see: https://retrace.fedoraproject.org/faf/problems/?component_names=gvfs&binary_names=%2Fusr%2Flibexec%2Fgvfsd-network https://bugzilla.redhat.com/show_bug.cgi?id=1402350 https://bugzilla.redhat.com/show_bug.cgi?id=1391518 https://bugzilla.redhat.com/show_bug.cgi?id=1191298
Created attachment 344210 [details] [review] network: Check variable before dereferencing It seems that the error variable may be NULL at this point in some cases as per the bug reports, but I don't really have any idea why. Let's check the error variable before dereferencing and see if it helps to reduce the number of bug reports...
Comment on attachment 344210 [details] [review] network: Check variable before dereferencing Attachment 344210 [details] pushed as d659151 - network: Check variable before dereferencing
I don't see why we'd want to do that instead of fixing the actual problem, I'd revert it. The daemon is monitoring non-local files, so this would be implemented in gdaemonfile.c in gvfs. And it looks like _g_daemon_vfs_get_mount_info_sync() will happily return NULL without setting an error: proxy = create_mount_tracker_proxy (); if (proxy == NULL) return NULL;
(In reply to Bastien Nocera from comment #3) > I don't see why we'd want to do that instead of fixing the actual problem, > I'd revert it. It is not instead of fixing the problem, it is just workaround before we find the real cause. I haven't seen any obvious place where it could fail. We can revert it once we fix it... > The daemon is monitoring non-local files, so this would be implemented in > gdaemonfile.c in gvfs. Yes, that's why I filed the bug against gvfs client :-) > And it looks like _g_daemon_vfs_get_mount_info_sync() will happily return > NULL without setting an error: > proxy = create_mount_tracker_proxy (); > if (proxy == NULL) > return NULL; Good catch! I will prepare patch for it...
Created attachment 344231 [details] [review] client: Propagate error from create_mount_tracker_proxy Prevent GDaemonFile methods failures without error being set. But still I am not sure that it fails on this place, because create_mount_tracker_proxy contains g_warning and I guess it should be in var_log_messages, but it isn't...
Is var_log_messages something other than /var/log/messages? Because that's not used on recent distros.
Review of attachment 344231 [details] [review]: Looks good.
(In reply to Bastien Nocera from comment #6) > Is var_log_messages something other than /var/log/messages? Because that's > not used on recent distros. I suppose so, because this file is also attached in downstream bug reports from abrt for recent Fedora versions, which doesn't use that file... but I will check with abrt people.
Comment on attachment 344231 [details] [review] client: Propagate error from create_mount_tracker_proxy Attachment 344231 [details] pushed as 6a62f4a - client: Propagate error from create_mount_tracker_proxy
(In reply to Ondrej Holy from comment #8) > (In reply to Bastien Nocera from comment #6) > > Is var_log_messages something other than /var/log/messages? Because that's > > not used on recent distros. > > I suppose so, because this file is also attached in downstream bug reports > from abrt for recent Fedora versions, which doesn't use that file... but I > will check with abrt people. They confirmed that it should contain journalctl output, see: https://github.com/abrt/abrt/blob/master/src/plugins/ccpp_event.conf#L32 I've made a test for sure and it really contains the warnings, so we are looking for something else here... :-/ any other tip? :-P
I've found the warning in "syslog" file finally, so attachment 344231 [details] [review] should be the proper fix.