Commit 8a9880ff authored by Colin Walters's avatar Colin Walters

Clean up inotify watch handling

Substantially based on a patch by Matthias Clasen <mclasen@redhat.com>
kqueue implementation by Joe Marcus Clarke <marcus@freebsd.org>

Previously, when we detected a configuration change (which included
the set of config directories to monitor for changes), we would
simply drop all watches, then readd them.

The problem with this is that it introduced a race condition where
we might not be watching one of the config directories for changes.

Rather than dropping and readding, change the OS-dependent monitoring
API to simply take a new set of directories to monitor.  Implicit
in this is that the OS-specific layer needs to keep track of the
previously monitored set.
parent 0705eb5c
......@@ -516,11 +516,6 @@ process_config_every_time (BusContext *context,
goto failed;
}
/* Drop existing conf-dir watches (if applicable) */
if (is_reload)
bus_drop_all_directory_watches ();
_DBUS_ASSERT_ERROR_IS_CLEAR (error);
retval = TRUE;
......@@ -551,13 +546,17 @@ process_config_postinit (BusContext *context,
_dbus_hash_table_unref (service_context_table);
/* Watch all conf directories */
_dbus_list_foreach (bus_config_parser_get_conf_dirs (parser),
(DBusForeachFunction) bus_watch_directory,
context);
bus_set_watched_dirs (context, bus_config_parser_get_conf_dirs (parser));
return TRUE;
}
static void
bus_shutdown_all_directory_watches (void *data)
{
bus_set_watched_dirs ((BusContext *) data, NULL);
}
BusContext*
bus_context_new (const DBusString *config_file,
ForceForkSetting force_fork,
......@@ -588,7 +587,9 @@ bus_context_new (const DBusString *config_file,
context->refcount = 1;
_dbus_generate_uuid (&context->uuid);
_dbus_register_shutdown_func (bus_shutdown_all_directory_watches, context);
if (!_dbus_string_copy_data (config_file, &context->config_file))
{
BUS_SET_OOM (error);
......
......@@ -34,6 +34,7 @@
#include <errno.h>
#include <dbus/dbus-internals.h>
#include <dbus/dbus-list.h>
#include <dbus/dbus-watch.h>
#include "dir-watch.h"
......@@ -43,6 +44,7 @@
/* use a static array to avoid handling OOM */
static int wds[MAX_DIRS_TO_WATCH];
static char *dirs[MAX_DIRS_TO_WATCH];
static int num_wds = 0;
static int inotify_fd = -1;
static DBusWatch *watch = NULL;
......@@ -90,12 +92,10 @@ _handle_inotify_watch (DBusWatch *passed_watch, unsigned int flags, void *data)
return TRUE;
}
void
bus_watch_directory (const char *dir, BusContext *context)
static int
_init_inotify (BusContext *context)
{
int wd;
_dbus_assert (dir != NULL);
int ret = 0;
if (inotify_fd == -1) {
#ifdef HAVE_INOTIFY_INIT1
......@@ -112,59 +112,117 @@ bus_watch_directory (const char *dir, BusContext *context)
watch = _dbus_watch_new (inotify_fd, DBUS_WATCH_READABLE, TRUE,
_handle_inotify_watch, NULL, NULL);
if (watch == NULL)
{
_dbus_warn ("Unable to create inotify watch\n");
goto out;
}
if (!_dbus_loop_add_watch (loop, watch, _inotify_watch_callback,
NULL, NULL))
{
_dbus_warn ("Unable to add reload watch to main loop");
_dbus_watch_unref (watch);
watch = NULL;
goto out;
}
if (watch == NULL)
{
_dbus_warn ("Unable to create inotify watch\n");
goto out;
}
if (!_dbus_loop_add_watch (loop, watch, _inotify_watch_callback,
NULL, NULL))
{
_dbus_warn ("Unable to add reload watch to main loop");
_dbus_watch_unref (watch);
watch = NULL;
goto out;
}
}
if (num_wds >= MAX_DIRS_TO_WATCH )
ret = 1;
out:
return ret;
}
void
bus_set_watched_dirs (BusContext *context, DBusList **directories)
{
int new_wds[MAX_DIRS_TO_WATCH];
char *new_dirs[MAX_DIRS_TO_WATCH];
DBusList *link;
int i, j, wd;
if (!_init_inotify (context))
goto out;
for (i = 0; i < MAX_DIRS_TO_WATCH; i++)
{
_dbus_warn ("Cannot watch config directory '%s'. Already watching %d directories\n", dir, MAX_DIRS_TO_WATCH);
goto out;
new_wds[i] = -1;
new_dirs[i] = NULL;
}
wd = inotify_add_watch (inotify_fd, dir, IN_CLOSE_WRITE | IN_DELETE | IN_MOVED_TO | IN_MOVED_FROM);
if (wd < 0)
i = 0;
link = _dbus_list_get_first_link (directories);
while (link != NULL)
{
_dbus_warn ("Cannot setup inotify for '%s'; error '%s'\n", dir, _dbus_strerror (errno));
goto out;
new_dirs[i++] = (char *)link->data;
link = _dbus_list_get_next_link (directories, link);
}
wds[num_wds++] = wd;
_dbus_verbose ("Added watch on config directory '%s'\n", dir);
out:
;
}
/* Look for directories in both the old and new sets, if
* we find one, move its data into the new set.
*/
for (i = 0; new_dirs[i]; i++)
{
for (j = 0; j < num_wds; j++)
{
if (dirs[j] && strcmp (new_dirs[i], dirs[j]) == 0)
{
new_wds[i] = wds[j];
new_dirs[i] = dirs[j];
wds[j] = -1;
dirs[j] = NULL;
break;
}
}
}
void
bus_drop_all_directory_watches (void)
{
int ret;
/* Any directories we find in "wds" with a nonzero fd must
* not be in the new set, so perform cleanup now.
*/
for (j = 0; j < num_wds; j++)
{
if (wds[j] != -1)
{
inotify_rm_watch (inotify_fd, wds[j]);
dbus_free (dirs[j]);
wds[j] = -1;
dirs[j] = NULL;
}
}
if (watch != NULL)
for (i = 0; new_dirs[i]; i++)
{
_dbus_loop_remove_watch (loop, watch, _inotify_watch_callback, NULL);
_dbus_watch_unref (watch);
watch = NULL;
if (new_wds[i] == -1)
{
/* FIXME - less lame error handling for failing to add a watch; we may need to sleep. */
wd = inotify_add_watch (inotify_fd, new_dirs[i], IN_CLOSE_WRITE | IN_DELETE | IN_MOVED_TO | IN_MOVED_FROM);
if (wd < 0)
{
_dbus_warn ("Cannot setup inotify for '%s'; error '%s'\n", new_dirs[i], _dbus_strerror (errno));
goto out;
}
new_wds[i] = wd;
new_dirs[i] = _dbus_strdup (new_dirs[i]);
if (!new_dirs[i])
{
/* FIXME have less lame handling for OOM, we just silently fail to
* watch. (In reality though, the whole OOM handling in dbus is stupid
* but we won't go into that in this comment =) )
*/
inotify_rm_watch (inotify_fd, wd);
new_wds[i] = -1;
}
}
}
_dbus_verbose ("Dropping all watches on config directories\n");
ret = close (inotify_fd);
if (ret)
_dbus_verbose ("Error dropping watches: '%s'\n", _dbus_strerror(errno));
num_wds = i;
for (i = 0; i < MAX_DIRS_TO_WATCH; i++)
{
wds[i] = new_wds[i];
dirs[i] = new_dirs[i];
}
num_wds = 0;
inotify_fd = -1;
out:;
}
......@@ -37,12 +37,14 @@
#include <dbus/dbus-watch.h>
#include <dbus/dbus-internals.h>
#include <dbus/dbus-list.h>
#include "dir-watch.h"
#define MAX_DIRS_TO_WATCH 128
static int kq = -1;
static int fds[MAX_DIRS_TO_WATCH];
static char *dirs[MAX_DIRS_TO_WATCH];
static int num_fds = 0;
static DBusWatch *watch = NULL;
static DBusLoop *loop = NULL;
......@@ -90,13 +92,10 @@ _handle_kqueue_watch (DBusWatch *watch, unsigned int flags, void *data)
return TRUE;
}
void
bus_watch_directory (const char *dir, BusContext *context)
static int
_init_kqueue (BusContext *context)
{
int fd;
struct kevent ev;
_dbus_assert (dir != NULL);
int ret = 0;
if (kq < 0)
{
......@@ -133,49 +132,114 @@ bus_watch_directory (const char *dir, BusContext *context)
}
}
if (num_fds >= MAX_DIRS_TO_WATCH )
ret = 1;
out:
return ret;
}
void
bus_set_watched_dir (BusContext *context, DBusList **directories)
{
int new_fds[MAX_DIRS_TO_WATCH];
char *new_dirs[MAX_DIRS_TO_WATCH];
DBusList *link;
int i, f, fd;
if (!_init_kqueue (context))
goto out;
for (i = 0; i < MAX_DIRS_TO_WATCH; i++) {
{
_dbus_warn ("Cannot watch config directory '%s'. Already watching %d directories\n", dir, MAX_DIRS_TO_WATCH);
goto out;
new_fds[i] = -1;
new_dirs[i] = NULL;
}
fd = open (dir, O_RDONLY);
if (fd < 0)
i = 0;
link = _dbus_list_get_first_link (directories);
while (link != NULL)
{
_dbus_warn ("Cannot open directory '%s'; error '%s'\n", dir, _dbus_strerror (errno));
goto out;
new_dirs[i++] = (char *)link->data;
link = _dbus_list_get_next_link (directories, link);
}
EV_SET (&ev, fd, EVFILT_VNODE, EV_ADD | EV_ENABLE | EV_CLEAR,
NOTE_DELETE | NOTE_EXTEND | NOTE_WRITE | NOTE_RENAME, 0, 0);
if (kevent (kq, &ev, 1, NULL, 0, NULL) == -1)
/* Look for directories in both the old and new sets, if
* we find one, move its data into the new set.
*/
for (i = 0; new_dirs[i]; i++)
{
_dbus_warn ("Cannot setup a kevent for '%s'; error '%s'\n", dir, _dbus_strerror (errno));
close (fd);
goto out;
for (j = 0; i < num_fds; j++)
{
if (dirs[j] && strcmp (new_dirs[i], dirs[j]) == 0)
{
new_fds[i] = fds[j];
new_dirs[i] = dirs[j];
fds[j] = -1;
dirs[j] = NULL;
break;
}
}
}
fds[num_fds++] = fd;
_dbus_verbose ("Added kqueue watch on config directory '%s'\n", dir);
out:
;
}
void
bus_drop_all_directory_watches (void)
{
int i;
_dbus_verbose ("Dropping all watches on config directories\n");
/* Any directory we find in "fds" with a nonzero fd must
* not be in the new set, so perform cleanup now.
*/
for (j = 0; j < num_fds; j++)
{
if (fds[j] != -1)
{
close (fds[j]);
dbus_free (dirs[j]);
fds[j] = -1;
dirs[j] = NULL;
}
}
for (i = 0; i < num_fds; i++)
for (i = 0; new_dirs[i]; i++)
{
if (close (fds[i]) != 0)
if (new_fds[i] == -1)
{
_dbus_verbose ("Error closing fd %d for config directory watch\n", fds[i]);
/* FIXME - less lame error handling for failing to add a watch;
* we may need to sleep.
*/
fd = open (new_dirs[i], O_RDONLY);
if (fd < 0)
{
_dbus_warn ("Cannot open directory '%s'; error '%s'\n", new_dirs[i], _dbus_strerror (errno));
goto out;
}
EV_SET (&ev, fd, EVFILT_VNODE, EV_ADD | EV_ENABLE | EV_CLEAR,
NOTE_DELETE | NOTE_EXTEND | NOTE_WRITE | NOTE_RENAME, 0, 0);
if (kevent (kq, &ev, 1, NULL, 0, NULL) == -1)
{
_dbus_warn ("Cannot setup a kevent for '%s'; error '%s'\n", dir, _dbus_strerror (errno));
close (fd);
goto out;
}
new_fds[i] = fd;
new_dirs[i] = _dbus_strdup (new_dirs[i]);
if (!new_dirs[i])
{
/* FIXME have less lame handling for OOM, we just silently fail to
* watch. (In reality though, the whole OOM handling in dbus is
* stupid but we won't go into that in this comment =) )
*/
close (fd);
new_fds[i] = -1;
}
}
}
num_fds = 0;
num_fds = i;
for (i = 0; i < MAX_DIRS_TO_WATCH; i++)
{
fds[i] = new_fds[i];
dirs[i] = new_dirs[i];
}
out:
;
}
......@@ -26,10 +26,15 @@
#ifndef DIR_WATCH_H
#define DIR_WATCH_H
/* setup a watch on a directory (OS dependent, may be a NOP) */
void bus_watch_directory (const char *directory, BusContext *context);
/* drop all the watches previously set up by bus_config_watch_directory (OS dependent, may be a NOP) */
void bus_drop_all_directory_watches (void);
/**
* Update the set of directories to monitor for changes. The
* operating-system-specific implementation of this function should
* avoid creating a window where a directory in both the
* old and new set isn't monitored.
*
* @param context The bus context
* @param dirs List of strings which are directory paths
*/
void bus_set_watched_dirs (BusContext *context, DBusList **dirs);
#endif /* DIR_WATCH_H */
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment