Bug 16294 - [PATCH] config not reloaded in future if one config file fails to load
Summary: [PATCH] config not reloaded in future if one config file fails to load
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.2.x
Hardware: Other All
: medium normal
Assignee: Havoc Pennington
QA Contact: John (J5) Palmieri
Depends on:
Reported: 2008-06-10 10:21 UTC by Dan Williams
Modified: 2008-07-28 09:03 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:

fix inotify (1.55 KB, patch)
2008-06-10 11:54 UTC, Colin Walters
Details | Splinter Review

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.