Bug 16294

Summary:	[PATCH] config not reloaded in future if one config file fails to load
Product:	dbus	Reporter:	Dan Williams <dcbw>
Component:	core	Assignee:	Havoc Pennington <hp>
Status:	RESOLVED FIXED	QA Contact:	John (J5) Palmieri <johnp>
Severity:	normal
Priority:	medium	CC:	walters
Version:	1.2.x
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	fix inotify

Description Dan Williams 2008-06-10 10:21:05 UTC

For some reason, sometimes when 'make install'-ing a project that installs dbus config files, dbus will fail to validate one of the config files and not re-watch the config directories.

- intofiy notices change, writes HUP to pipe
- mainloop processes HUP, calls bus_context_reload_config()
- process_config_every_time() returns an error, possibly it tried to read in a half-written file
- bus_context_reload_config() skips process_config_postinit(), which re-watches the config directory

All further updates of config files in the config directory are lost until a SIGHUP is sent to the daemon.

I'd suggest unconditionally calling process_config_postinit() or unconditionally re-adding the directory watches even if process_config_every_time() fails.

Comment 1 Colin Walters 2008-06-10 11:54:21 UTC

Created attachment 17034 [details] [review]
fix inotify

Dan and I tracked this down; in handle_inotify_watch, we *always* remove the watch from the mainloop, even if no error occurred.  This caused a side effect where we were relying on then later calling bus_drop_all_directory_watches which would close the fd, and cause the watch to be readded when we first process a directory.

In the case where we hit a config file error, we would drop the watch, but then it would never be readded.

The right fix here I believe is to drop the watch inside bus_drop_all_directory_watches to match the inotify fd being closed and set to -1.

I checked the FreeBSD kqueue version, and they only drop the watch on error.

I'm almost sure this change is right because the code to drop the watch was clearly wrong - it set the watch to NULL even though the watch parameter was an auto parameter that shadowed the global watch variable.

Comment 2 Colin Walters 2008-07-28 09:03:20 UTC

commit 121c6b13a35759ee590551cfb9978e9f20766a12
Author: Colin Walters <walters@verbum.org>
Date:   Mon Jul 28 12:02:56 2008 -0400

    Bug 16294: Don't lose inotify watch when config fails to parse
    
    	* bus/dir-watch-inotify.c: Always drop the watch in
    	handle_inotify_watch; this ensures we always readd it
    	correctly in bus_drop_all_directory_watches.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.