diff --git a/cf-reactor/file_watcher.c b/cf-reactor/file_watcher.c index 8a82d06dff1..869df2d7cab 100644 --- a/cf-reactor/file_watcher.c +++ b/cf-reactor/file_watcher.c @@ -30,114 +30,141 @@ #include #include -// =========== code for EVENT_FILE_DELETED event type =========== - -/* Minimum time between two logs of a persisting lstat() failure, since the - * file is checked on every poll */ -#define LSTAT_ERROR_LOG_INTERVAL_SECS 60 +/* Minimum time between two logs of a persisting failure to check the file, + * since the file is checked on every poll */ +#define CHECK_ERROR_LOG_INTERVAL_SECS 60 typedef struct { char *path; - time_t last_seen; /* Time the file was last observed present, 0 if absent */ - time_t lstat_error_logged; /* Time an lstat() failure was last logged, 0 if lstat() works */ + time_t last_checked; /* Time of the last successful check, 0 if none yet */ + time_t last_error_logged; /* Time the last error was logged, 0 if no errors */ + + /* What the last successful check observed */ + bool last_exists; } FileWatcherState; -/* Sets *deleted to whether the path of the watcher exists. Only - * ENOENT/ENOTDIR mean the path genuinely doesn't exist. Any other lstat() - * failure (e.g. EACCES, ESTALE) is a transient/permission error, in which case - * we can't tell whether the file was deleted, so false is returned and - * *deleted is left untouched. A persisting failure is logged at most once - * every LSTAT_ERROR_LOG_INTERVAL_SECS. - * - * Doesn't follow symlinks, i.e. a dangling symlink is not considered deleted */ -static bool FileDeleted(FileWatcherState *fws, bool *deleted) + +void *FileWatcherStateNew(const Rlist *args) { - assert(fws != NULL); - assert(deleted != NULL); + assert(args != NULL); + const char *path = RlistScalarValue(args); - struct stat sb; - if (lstat(fws->path, &sb) == 0) - { - *deleted = false; - fws->lstat_error_logged = 0; - return true; - } + FileWatcherState *fws = (FileWatcherState *) xcalloc(1, sizeof(FileWatcherState)); + fws->path = xstrdup(path); - if (errno == ENOENT || errno == ENOTDIR) + return fws; +} + +void FileWatcherStateDestroy(void *state) +{ + FileWatcherState *fws = state; + if (fws != NULL) { - *deleted = true; - fws->lstat_error_logged = 0; - return true; + free(fws->path); + free(fws); } +} + +// =========== code shared by the file event types =========== + +/* Logs the failure of operation op (with errno err) on the file of the + * watcher. A persisting failure is logged at most once every + * CHECK_ERROR_LOG_INTERVAL_SECS. */ +static void LogCheckError(FileWatcherState *fws, const char *op, int err) +{ + assert(fws != NULL); - const int lstat_errno = errno; const time_t now = time(NULL); - if (fws->lstat_error_logged == 0 || - now - fws->lstat_error_logged >= LSTAT_ERROR_LOG_INTERVAL_SECS) + if (fws->last_error_logged == 0 || + now - fws->last_error_logged >= CHECK_ERROR_LOG_INTERVAL_SECS) { - Log(LOG_LEVEL_ERR, "Unable to lstat '%s' while checking for file deletion: %s", - fws->path, GetErrorStrFromCode(lstat_errno)); - fws->lstat_error_logged = now; + Log(LOG_LEVEL_ERR, "Unable to %s '%s' while checking for file events: %s", + op, fws->path, GetErrorStrFromCode(err)); + fws->last_error_logged = now; } - return false; } -void *FileWatcherStateNew(const char *path) +/* Checks whether the file of the watcher exists, with lstat(). On success, + * returns true and sets *exists, as well as *sb if the file exists and sb is + * not NULL. Only ENOENT/ENOTDIR mean the file doesn't exist. On any other + * lstat() failure (e.g. EACCES, ESTALE), whether the file exists is unknown, + * so the failure is logged (see LogCheckError()) and false is returned. + * + * Doesn't follow symlinks, e.g. a dangling symlink exists */ +static bool FileLstat(FileWatcherState *fws, bool *exists, struct stat *sb) { - FileWatcherState *fws = (FileWatcherState *) xmalloc(sizeof(FileWatcherState)); + assert(fws != NULL); + assert(exists != NULL); - fws->path = xstrdup(path); - fws->lstat_error_logged = 0; + struct stat buf; + if (lstat(fws->path, (sb != NULL) ? sb : &buf) == 0) + { + *exists = true; + fws->last_error_logged = 0; + return true; + } - /* If the file's presence can't be determined, treat it as not yet seen */ - bool deleted; - fws->last_seen = (FileDeleted(fws, &deleted) && !deleted) ? time(NULL) : 0; + if (errno == ENOENT || errno == ENOTDIR) + { + *exists = false; + fws->last_error_logged = 0; + return true; + } - return (void *) fws; + LogCheckError(fws, "lstat", errno); + return false; } -WatcherCheckResult FileWatcherCheckFileDeleted(void *state) +/* Records a successful check, at which the file existed or not and the event + * (e.g. "deletion") happened or not, and returns its result. The first + * successful check is only the reference for the next ones, so it can't + * fire. */ +static WatcherCheckResult FileWatcherRecordObservation(FileWatcherState *fws, bool exists, + bool happened, const char *event) { - assert(state != NULL); - FileWatcherState *fws = state; - - bool deleted_now; - if (!FileDeleted(fws, &deleted_now)) - { - /* Unknown state, keep last_seen as is */ - return WATCHER_CHECK_ERROR; - } + assert(fws != NULL); + assert(event != NULL); const time_t now = time(NULL); - const bool deleted = (fws->last_seen != 0) && deleted_now; + happened = happened && (fws->last_checked != 0); - if (deleted) + if (happened) { - char last_seen_str[26]; - const char *last_seen_ts = cf_strtimestamp_local(fws->last_seen, last_seen_str); + char last_checked_str[26]; + const char *last_checked_ts = cf_strtimestamp_local(fws->last_checked, last_checked_str); char now_str[26]; const char *now_ts = cf_strtimestamp_local(now, now_str); Log(LOG_LEVEL_VERBOSE, - "Observed deletion of file '%s' at %s (last seen present at %s)", + "Observed %s of file '%s' at %s (last checked at %s)", + event, fws->path, (now_ts != NULL) ? now_ts : "unknown time", - (last_seen_ts != NULL) ? last_seen_ts : "unknown time"); + (last_checked_ts != NULL) ? last_checked_ts : "unknown time"); } - fws->last_seen = deleted_now ? 0 : now; - return deleted ? WATCHER_CHECK_EVENT : WATCHER_CHECK_NO_EVENT; + fws->last_checked = now; + fws->last_exists = exists; + return happened ? WATCHER_CHECK_EVENT : WATCHER_CHECK_NO_EVENT; } -void DestroyFileWatcherState(void *state) +// =========== code for the file_deleted event =========== + +WatcherCheckResult FileWatcherCheckFileDeleted(void *state) { + assert(state != NULL); FileWatcherState *fws = state; - if (fws != NULL) + + bool exists; + if (!FileLstat(fws, &exists, NULL)) { - free(fws->path); - free(fws); + /* Unknown state, keep the last observation as is */ + return WATCHER_CHECK_ERROR; } + + bool deleted = fws->last_exists && !exists; + return FileWatcherRecordObservation(fws, exists, deleted, "deletion"); } diff --git a/cf-reactor/file_watcher.h b/cf-reactor/file_watcher.h index f96ea8daae5..2ab0eaa5cd5 100644 --- a/cf-reactor/file_watcher.h +++ b/cf-reactor/file_watcher.h @@ -29,8 +29,8 @@ #include /* WatcherCheckResult */ -void *FileWatcherStateNew(const char *path); +void *FileWatcherStateNew(const Rlist *args); WatcherCheckResult FileWatcherCheckFileDeleted(void *state); -void DestroyFileWatcherState(void *state); +void FileWatcherStateDestroy(void *state); #endif diff --git a/cf-reactor/reactor_transform.c b/cf-reactor/reactor_transform.c index 27c725a46fb..cc0119bf70c 100644 --- a/cf-reactor/reactor_transform.c +++ b/cf-reactor/reactor_transform.c @@ -33,13 +33,16 @@ #include #include #include -#include #include // ScheduleAgentOperations() #include // GetClassContextAttributes() #include // ConnCache_Init(), ConnCache_Destroy() #include // Initialize/FinalizeCustomPromises() #include // SetReferenceTime() #include // UpdateTimeClasses() +#include // REACTOR_INTERVAL_RANGE, REACTOR_FILE_DELETED +#include // CheckConstraintTypeMatch(), PromiseTypeSyntaxGet(), BodySyntaxGetConstraintSyntax() +#include +#include /* Promise types evaluated within `bundle reactor NAME { ... }`. */ static const char *const REACTOR_TYPESEQUENCE[] = @@ -134,22 +137,213 @@ static const Rlist *GetThenBundles(const Promise *pp) return bundles; } -// Temporary limitations: -// - 'when' body: only a single constraint, 'file_deleted', is supported (no OR-ing of constraints yet) -// TODO: support multiple 'when' constraints -static PromiseResult KeepEventsPromise(EvalContext *ctx, const Promise *pp) +/*****************************************************************************/ + +/* A constraint of the 'when' body passed to the watcher of an event, with the + * type of value it accepts */ +typedef struct +{ + const char *name; + RvalType type; +} EventArg; + +typedef struct +{ + const char *name; + RvalType type; /* type of value the 'name' constraint accepts */ + const EventArg *extra_args; /* terminated by an EventArg with a NULL name */ + WatcherStateNewFn new_state; + WatcherStateDestroyFn destroy_state; + WatcherCheckFn check; +} EventType; + +static const EventArg NO_EXTRA_ARGS[] = +{ + { NULL, RVAL_TYPE_NOPROMISEE } +}; + +#define EventTypeNew(name, type, args, new_state, destroy_state, check) \ + { name, type, args, new_state, destroy_state, check } +#define EventTypeNewNull() EventTypeNew(NULL, RVAL_TYPE_NOPROMISEE, NULL, NULL, NULL, NULL) + +/* A file event, about the file at the path of its constraint */ +#define FileEventTypeNew(name, check) \ + EventTypeNew(name, RVAL_TYPE_SCALAR, NO_EXTRA_ARGS, FileWatcherStateNew, FileWatcherStateDestroy, check) + +static const EventType EVENT_TABLE[] = +{ + FileEventTypeNew(REACTOR_FILE_DELETED, FileWatcherCheckFileDeleted), + EventTypeNewNull() +}; + +/* Appends the value of constraint con of the events promise to args if it is + * of the given type, otherwise logs an error and returns false. */ +static bool EventArgAppend(const Promise *pp, const Constraint *con, RvalType type, Rlist **args) +{ + assert(pp != NULL); + assert(con != NULL); + assert(args != NULL); + + if (con->rval.type != type) + { + Log(LOG_LEVEL_ERR, + "Reactor events promise '%s' has an invalid '%s', expected a %s value, got a %s value", + pp->promiser, con->lval, RvalTypeToString(type), RvalTypeToString(con->rval.type)); + return false; + } + RlistAppend(args, con->rval.item, con->rval.type); + return true; +} + +static const EventType *EventsPromiseGetFirstEvent(const Promise *pp, Rlist **args) { assert(pp != NULL); + assert(args != NULL); - const char *path = PromiseGetConstraintAsRval(pp, "file_deleted", RVAL_TYPE_SCALAR); - if (path == NULL) + for (size_t i = 0; EVENT_TABLE[i].name != NULL; i++) { - Log(LOG_LEVEL_WARNING, - "Reactor events promise '%s' must specify exactly one file to watch in its 'when' body, ignoring", + const Constraint *con = PromiseGetConstraint(pp, EVENT_TABLE[i].name); + if (con != NULL) + { + if (!EventArgAppend(pp, con, EVENT_TABLE[i].type, args)) + { + return NULL; + } + return &EVENT_TABLE[i]; + } + } + Log(LOG_LEVEL_ERR, + "Reactor events promise '%s' does not specify an event to watch in its 'when' body", + pp->promiser); + return NULL; +} + +#define DEFAULT_INTERVAL 1 + +/* Sets *interval to the seconds between two checks for the event of the + * (expanded) events promise. The parser and cf-promises only check it if it + * doesn't depend on variables or body arguments, so it is checked again here + * the same way: logs an error and returns false if it is invalid. */ +static bool EventsPromiseGetInterval(const Promise *pp, time_t *interval) +{ + assert(pp != NULL); + assert(interval != NULL); + + const Constraint *interval_con = PromiseGetConstraint(pp, "interval"); + if (interval_con == NULL) + { + *interval = DEFAULT_INTERVAL; + return true; + } + + if (interval_con->rval.type != RVAL_TYPE_SCALAR) + { + Log(LOG_LEVEL_ERR, + "Reactor events promise '%s' has an invalid interval, expected a number of seconds", pp->promiser); - return PROMISE_RESULT_FAIL; + return false; + } + + const char *value = RvalScalarValue(interval_con->rval); + const SyntaxTypeMatch err = CheckConstraintTypeMatch( + interval_con->lval, interval_con->rval, CF_DATA_TYPE_INT, REACTOR_INTERVAL_RANGE, 0); + if (err != SYNTAX_TYPE_MATCH_OK) + { + Log(LOG_LEVEL_ERR, + "Reactor events promise '%s' has an invalid interval '%s': %s", + pp->promiser, value, SyntaxTypeMatchToString(err)); + return false; + } + + *interval = (time_t) IntFromString(value); + return true; +} + +static bool ResolveWhenBody(const Promise *pp, WatcherOptions *opt) +{ + assert(pp != NULL); + assert(opt != NULL); + // Get the event and its arg + Rlist *args = NULL; + const EventType *event = EventsPromiseGetFirstEvent(pp, &args); + + if (event == NULL) + { + RlistDestroy(args); + return false; + } + + // If the event has additional args, add them to the list of args + for (const EventArg *extra_arg = event->extra_args; extra_arg->name != NULL; extra_arg++) + { + const Constraint *con = PromiseGetConstraint(pp, extra_arg->name); + if (con == NULL) + { + Log(LOG_LEVEL_ERR, + "Constraint '%s' is required for event '%s' in events promise '%s'", + extra_arg->name, event->name, pp->promiser); + RlistDestroy(args); + return false; + } + if (!EventArgAppend(pp, con, extra_arg->type, &args)) + { + RlistDestroy(args); + return false; + } + } + + // Check that other constraints are not defined in the promise, that + // do not belong here + const PromiseTypeSyntax *events_syntax = PromiseTypeSyntaxGet("reactor", "events"); + assert(events_syntax != NULL); + const ConstraintSyntax *when_syntax = + PromiseTypeSyntaxGetConstraintSyntax(events_syntax, "when")->range.body_type_syntax->constraints; + + for (size_t i = 0; i < SeqLength(pp->conlist); i++) + { + const Constraint *cp = SeqAt(pp->conlist, i); + if (BodySyntaxGetConstraintSyntax(when_syntax, cp->lval) == NULL || /* not a 'when' constraint */ + StringEqual(cp->lval, event->name) || + StringEqual(cp->lval, "interval") || + StringEqual(cp->lval, "meta")) /* also a promise attribute */ + { + continue; + } + + bool own_arg = false; + for (const EventArg *extra_arg = event->extra_args; extra_arg->name != NULL; extra_arg++) + { + own_arg = own_arg || StringEqual(extra_arg->name, cp->lval); + } + if (!own_arg) + { + Log(LOG_LEVEL_ERR, + "Reactor events promise '%s' cannot have both '%s' and '%s' in its 'when' body, only one event can be watched per promise", + pp->promiser, event->name, cp->lval); + RlistDestroy(args); + return false; + } } + if (!EventsPromiseGetInterval(pp, &opt->interval)) + { + RlistDestroy(args); + return false; + } + + opt->state = event->new_state(args); + opt->check_callback = event->check; + opt->destroy_state = event->destroy_state; + RlistDestroy(args); + return true; +} + +/*****************************************************************************/ + +static PromiseResult KeepEventsPromise(EvalContext *ctx, const Promise *pp) +{ + assert(pp != NULL); + const Rlist *then_bundles = GetThenBundles(pp); if (then_bundles == NULL) { @@ -164,15 +358,16 @@ static PromiseResult KeepEventsPromise(EvalContext *ctx, const Promise *pp) } } - WatcherOptions opt = { - .state = FileWatcherStateNew(path), - .check_callback = FileWatcherCheckFileDeleted, - .destroy_state = DestroyFileWatcherState, - }; + WatcherOptions opt; + if (!ResolveWhenBody(pp, &opt)) + { + return PROMISE_RESULT_FAIL; + } - // register watcher - Log(LOG_LEVEL_INFO, "Registering a file_deleted watcher for events promise '%s', on file '%s'", pp->promiser, path); - WatcherRegister(pp, opt, 1); + if (!WatcherRegister(pp, &opt)) + { + return PROMISE_RESULT_FAIL; + } return PROMISE_RESULT_NOOP; } diff --git a/cf-reactor/watcher.c b/cf-reactor/watcher.c index dd2bc22436f..c72bd8ffb9c 100644 --- a/cf-reactor/watcher.c +++ b/cf-reactor/watcher.c @@ -119,56 +119,75 @@ void WatcherRegistryFinalize(void) } // Expects interval to be strictly greater than 0, otherwise the watcher thread will busy spin -void WatcherRegister(const Promise *pp, WatcherOptions opt, time_t interval) +bool WatcherRegister(const Promise *pp, WatcherOptions *opt) { assert(pp != NULL); assert(pp->promiser != NULL); - assert(opt.check_callback != NULL); - assert(opt.destroy_state != NULL); - assert(opt.state != NULL); + assert(opt != NULL); + assert(opt->check_callback != NULL); + assert(opt->destroy_state != NULL); + assert(opt->interval > 0); assert(watchers != NULL); - assert(interval > 0); - - ThreadLock(&watchers_mutex); + char promise_hash[CF_HOSTKEY_STRING_SIZE]; WatcherPromiseHash(pp, promise_hash, sizeof(promise_hash)); - Watcher *prev = MapGet(watchers, promise_hash); - if (prev != NULL) + ThreadLock(&watchers_mutex); + + if (MapHasKey(watchers, promise_hash)) { // ignore duplicates ThreadUnlock(&watchers_mutex); - opt.destroy_state(opt.state); - return; + opt->destroy_state(opt->state); + return true; } - Watcher *w = xmalloc(sizeof(Watcher)); - w->promise_hash = xstrdup(promise_hash); - w->promise = pp->org_pp; - w->promiser = xstrdup(pp->promiser); - w->poll_interval_secs = interval; - w->next_due = 0; /* due immediately on the watcher thread's first pass */ - w->last_check = WATCHER_CHECK_NO_EVENT; - w->check_callback = opt.check_callback; - w->destroy_state = opt.destroy_state; - // Take over the state of an identical watcher - prev = (old_watchers != NULL) ? MapGet(old_watchers, promise_hash) : NULL; - if (prev == NULL) + Watcher *prev = (old_watchers != NULL) ? MapGet(old_watchers, promise_hash) : NULL; + const bool taken_over = (prev != NULL); + void *state = opt->state; + if (taken_over) + { + state = prev->state; + prev->state = NULL; /* so not destroyed with prev */ + } + + ThreadUnlock(&watchers_mutex); + + if (taken_over) { - w->state = opt.state; + Log(LOG_LEVEL_DEBUG, "Reactor watcher '%s' takes over the state of an identical watcher", pp->promiser); + opt->destroy_state(opt->state); } else { - Log(LOG_LEVEL_DEBUG, "Reactor watcher '%s' takes over the state of an identical watcher", w->promiser); - w->state = prev->state; - prev->state = NULL; /* so not destroyed with prev */ - opt.destroy_state(opt.state); + /* Without holding watchers_mutex, as the check may block (e.g. + * lstat() on an unresponsive NFS mount). The watcher isn't registered + * yet, so the watcher thread can't check it concurrently. */ + if (opt->check_callback(state) == WATCHER_CHECK_ERROR) + { + /* No reference for the next checks, so not registered */ + opt->destroy_state(state); + return false; + } } - MapInsert(watchers, w->promise_hash, w); + Watcher *w = xmalloc(sizeof(Watcher)); + w->promise_hash = xstrdup(promise_hash); + w->promise = pp->org_pp; + w->promiser = xstrdup(pp->promiser); + w->poll_interval_secs = opt->interval; + w->next_due = 0; /* due immediately on the watcher thread's first pass */ + w->last_check = WATCHER_CHECK_NO_EVENT; + w->check_callback = opt->check_callback; + w->destroy_state = opt->destroy_state; + w->state = state; + ThreadLock(&watchers_mutex); + MapInsert(watchers, w->promise_hash, w); ThreadUnlock(&watchers_mutex); + + return true; } static void WatcherDestroy(void *item) @@ -304,7 +323,7 @@ static void HandleQueuedEvents(EvalContext *ctx, WatcherEventFn on_event) } else { - Log(LOG_LEVEL_NOTICE, "Reactor watcher '%s' fired", w->promiser); + Log(LOG_LEVEL_VERBOSE, "Reactor watcher '%s' fired", w->promiser); } on_event(ctx, w->promise, w->promise_hash, event->check); free(event); diff --git a/cf-reactor/watcher.h b/cf-reactor/watcher.h index 4f14efb14d3..60134096091 100644 --- a/cf-reactor/watcher.h +++ b/cf-reactor/watcher.h @@ -35,6 +35,7 @@ typedef enum WATCHER_CHECK_ERROR, /* unable to tell whether the event happened */ } WatcherCheckResult; +typedef void *(*WatcherStateNewFn)(const Rlist *args); typedef WatcherCheckResult (*WatcherCheckFn)(void *state); typedef void (*WatcherStateDestroyFn)(void *state); @@ -46,6 +47,7 @@ typedef struct WatcherCheckFn check_callback; /* whether the event happened, updating state */ WatcherStateDestroyFn destroy_state; void *state; /* owned by the registry once registered */ + time_t interval; /* seconds between two checks, > 0 */ } WatcherOptions; /** @@ -85,18 +87,23 @@ void WatcherRegistryFinalize(void); * * If a watcher of the previous policy (see EventWatcherPause()) was * registered for an identical expanded promise, the new watcher takes over - * its state instead of opt.state, so that the events happening while paused - * are detected. + * its state instead of opt->state, so that the events happening while paused + * are detected. Otherwise opt->state is checked once right away, as the + * reference for the next checks, so that only what happens from now on + * fires. If that check fails, the watcher is not registered. * * @param pp the expanded events promise. Its hash and its unexpanded * promise (pp->org_pp) are passed back to the WatcherEventFn on * event. The unexpanded promise is not owned, so the policy * holding it must not be destroyed before EventWatcherPause(). - * @param opt what the watcher checks for, see WatcherOptions. opt.state is - * owned by the registry. - * @param interval interval between runs + * @param opt what the watcher checks for, see WatcherOptions. opt->state is + * owned by the registry, which destroys it right away if not + * used, so it must not be used by the caller afterwards. + * opt->interval must be strictly greater than 0. + * @return false if the check of opt->state failed (WATCHER_CHECK_ERROR), in + * which case the watcher is not registered, true otherwise. */ -void WatcherRegister(const Promise *pp, WatcherOptions opt, time_t interval); +bool WatcherRegister(const Promise *pp, WatcherOptions *opt); bool EventWatcherInitialize(int *fd); void EventWatcherHandleEvents(EvalContext *ctx, WatcherEventFn on_event, int fd, fd_set *readfds); diff --git a/libpromises/mod_reactor.c b/libpromises/mod_reactor.c index 113a3e9a1a8..520d28f03ef 100644 --- a/libpromises/mod_reactor.c +++ b/libpromises/mod_reactor.c @@ -32,7 +32,8 @@ static const ConstraintSyntax when_constraints[] = CONSTRAINT_SYNTAX_GLOBAL, /* Row models */ - ConstraintSyntaxNewString("file_deleted", CF_ANYSTRING, "File to react for on deletion", SYNTAX_STATUS_NORMAL), + ConstraintSyntaxNewString(REACTOR_FILE_DELETED, CF_ANYSTRING, "File to react for on deletion", SYNTAX_STATUS_NORMAL), + ConstraintSyntaxNewInt("interval", REACTOR_INTERVAL_RANGE, "Seconds between two checks for the event", SYNTAX_STATUS_NORMAL), ConstraintSyntaxNewNull() }; diff --git a/libpromises/mod_reactor.h b/libpromises/mod_reactor.h index 72842f35045..d84ed22643c 100644 --- a/libpromises/mod_reactor.h +++ b/libpromises/mod_reactor.h @@ -27,6 +27,11 @@ #include +#define REACTOR_FILE_DELETED "file_deleted" + +/* Range of the 'interval' attribute of the 'when' body of events promises */ +#define REACTOR_INTERVAL_RANGE "1,99999999999" + extern const PromiseTypeSyntax CF_REACTOR_PROMISE_TYPES[]; #endif diff --git a/tests/unit/watcher_test.c b/tests/unit/watcher_test.c index c8255195317..3573c91d709 100644 --- a/tests/unit/watcher_test.c +++ b/tests/unit/watcher_test.c @@ -3,6 +3,7 @@ #include #include /* FileWatcherStateNew(), FileWatcherCheckFileDeleted() */ #include /* Promise */ +#include /* RlistAppendScalar() */ #include /* LoggingPrivContext, LoggingPrivSetContext() */ #include /* strstr() */ @@ -34,14 +35,22 @@ static const Promise expanded_promise = { .org_pp = &unexpanded_promise, }; -static WatcherOptions FileDeletedOptions(const char *path) +/* Returns the options of a file_deleted watcher, valid until the next call + * (WatcherRegister() copies them) */ +static WatcherOptions *FileDeletedOptions(const char *path, time_t interval) { - WatcherOptions opt = { - .state = FileWatcherStateNew(path), + static WatcherOptions opt; + + Rlist *args = NULL; + RlistAppendScalar(&args, path); + opt = (WatcherOptions) { + .state = FileWatcherStateNew(args), .check_callback = FileWatcherCheckFileDeleted, - .destroy_state = DestroyFileWatcherState, + .destroy_state = FileWatcherStateDestroy, + .interval = interval, }; - return opt; + RlistDestroy(args); + return &opt; } /* Creates the file 'file' in a new temporary directory dir */ @@ -121,7 +130,7 @@ static void test_watcher_register_single(void) { WatcherRegistryInitialize(); - WatcherRegister(&expanded_promise, FileDeletedOptions("/nonexistent/test-event"), 5); + assert_true(WatcherRegister(&expanded_promise, FileDeletedOptions("/nonexistent/test-event", 5))); WatcherRegistryFinalize(); } @@ -134,8 +143,8 @@ static void test_watcher_register_iterations(void) * different watchers */ const Promise iteration_a = { .promiser = "/a", .org_pp = &unexpanded_promise }; const Promise iteration_b = { .promiser = "/b", .org_pp = &unexpanded_promise }; - WatcherRegister(&iteration_a, FileDeletedOptions("/nonexistent/a"), 5); - WatcherRegister(&iteration_b, FileDeletedOptions("/nonexistent/b"), 5); + WatcherRegister(&iteration_a, FileDeletedOptions("/nonexistent/a", 5)); + WatcherRegister(&iteration_b, FileDeletedOptions("/nonexistent/b", 5)); WatcherRegistryFinalize(); } @@ -157,8 +166,8 @@ static void test_watcher_register_identical_ignored(void) .promiser = "test-event", .org_pp = &other_unexpanded_promise, }; - WatcherRegister(&expanded_promise, FileDeletedOptions(path), 1); - WatcherRegister(&other_expanded_promise, FileDeletedOptions(path), 1); + assert_true(WatcherRegister(&expanded_promise, FileDeletedOptions(path, 1))); + assert_true(WatcherRegister(&other_expanded_promise, FileDeletedOptions(path, 1))); int fd = -1; assert_true(EventWatcherInitialize(&fd)); @@ -185,18 +194,18 @@ static void test_watcher_register_again_after_pause(void) * registered again for the new policy, also without the event watcher * being initialized */ ResetRecordedEvents(); - WatcherRegister(&expanded_promise, FileDeletedOptions("/nonexistent/test-event"), 5); + WatcherRegister(&expanded_promise, FileDeletedOptions("/nonexistent/test-event", 5)); EventWatcherPause(NULL, RecordEvent); - WatcherRegister(&expanded_promise, FileDeletedOptions("/nonexistent/test-event"), 5); + WatcherRegister(&expanded_promise, FileDeletedOptions("/nonexistent/test-event", 5)); EventWatcherResume(); /* Paused twice in a row (e.g. a failed policy reload in between), the * watchers set aside the first time are replaced by those registered * since, without leaking (checked by valgrind/ASan) */ EventWatcherPause(NULL, RecordEvent); - WatcherRegister(&expanded_promise, FileDeletedOptions("/nonexistent/test-event"), 5); + WatcherRegister(&expanded_promise, FileDeletedOptions("/nonexistent/test-event", 5)); EventWatcherPause(NULL, RecordEvent); - WatcherRegister(&expanded_promise, FileDeletedOptions("/nonexistent/test-event"), 5); + WatcherRegister(&expanded_promise, FileDeletedOptions("/nonexistent/test-event", 5)); EventWatcherResume(); assert_int_equal(handled_event_count, 0); @@ -238,7 +247,7 @@ static void test_event_watcher_pause_handles_queued_events(void) CreateTempFile(dir, path, sizeof(path)); WatcherRegistryInitialize(); - WatcherRegister(&expanded_promise, FileDeletedOptions(path), 1); + WatcherRegister(&expanded_promise, FileDeletedOptions(path, 1)); int fd = -1; assert_true(EventWatcherInitialize(&fd)); @@ -276,7 +285,7 @@ static void test_event_watcher_idle_woken_by_wake_up(void) sleep(1); /* Registering a watcher doesn't wake it up, EventWatcherWakeUp() must */ - WatcherRegister(&expanded_promise, FileDeletedOptions(path), 1); + WatcherRegister(&expanded_promise, FileDeletedOptions(path, 1)); assert_int_equal(unlink(path), 0); assert_false(WaitForQueuedEvent(fd, 20)); EventWatcherWakeUp(); @@ -304,7 +313,7 @@ static void test_event_watcher_idle_woken_by_resume(void) /* Registering doesn't wake the watcher thread up, resuming must */ EventWatcherPause(NULL, RecordEvent); - WatcherRegister(&expanded_promise, FileDeletedOptions(path), 1); + WatcherRegister(&expanded_promise, FileDeletedOptions(path, 1)); assert_int_equal(unlink(path), 0); assert_false(WaitForQueuedEvent(fd, 20)); EventWatcherResume(); @@ -326,7 +335,7 @@ static void test_event_watcher_state_kept_across_pause(void) CreateTempFile(dir, path, sizeof(path)); WatcherRegistryInitialize(); - WatcherRegister(&expanded_promise, FileDeletedOptions(path), 1); + WatcherRegister(&expanded_promise, FileDeletedOptions(path, 1)); int fd = -1; assert_true(EventWatcherInitialize(&fd)); @@ -346,7 +355,7 @@ static void test_event_watcher_state_kept_across_pause(void) .promiser = "test-event", .org_pp = &new_unexpanded_promise, }; - WatcherRegister(&new_expanded_promise, FileDeletedOptions(path), 1); + WatcherRegister(&new_expanded_promise, FileDeletedOptions(path, 1)); EventWatcherResume(); assert_true(WaitForQueuedEvent(fd, 50)); @@ -367,7 +376,7 @@ static void test_event_watcher_state_kept_identical_ignored(void) CreateTempFile(dir, path, sizeof(path)); WatcherRegistryInitialize(); - WatcherRegister(&expanded_promise, FileDeletedOptions(path), 1); + WatcherRegister(&expanded_promise, FileDeletedOptions(path, 1)); int fd = -1; assert_true(EventWatcherInitialize(&fd)); @@ -389,8 +398,8 @@ static void test_event_watcher_state_kept_identical_ignored(void) .promiser = "test-event", .org_pp = &other_unexpanded_promise, }; - WatcherRegister(&new_expanded_promise, FileDeletedOptions(path), 1); - WatcherRegister(&other_expanded_promise, FileDeletedOptions(path), 1); + WatcherRegister(&new_expanded_promise, FileDeletedOptions(path, 1)); + WatcherRegister(&other_expanded_promise, FileDeletedOptions(path, 1)); EventWatcherResume(); assert_true(WaitForQueuedEvent(fd, 50)); @@ -410,7 +419,7 @@ static void test_event_watcher_state_not_kept_for_other_promise(void) CreateTempFile(dir, path, sizeof(path)); WatcherRegistryInitialize(); - WatcherRegister(&expanded_promise, FileDeletedOptions(path), 1); + WatcherRegister(&expanded_promise, FileDeletedOptions(path, 1)); int fd = -1; assert_true(EventWatcherInitialize(&fd)); @@ -425,7 +434,7 @@ static void test_event_watcher_state_not_kept_for_other_promise(void) .promiser = "other-event", .org_pp = &unexpanded_promise, }; - WatcherRegister(&other_expanded_promise, FileDeletedOptions(path), 1); + WatcherRegister(&other_expanded_promise, FileDeletedOptions(path, 1)); EventWatcherResume(); /* The watcher thread checks at least every second */ @@ -463,9 +472,12 @@ static void test_check_file_deleted_error(void) const LogLevel old_level = LogGetGlobalLevel(); LogSetGlobalLevel(LOG_LEVEL_CRIT); - void *state = FileWatcherStateNew(path); + Rlist *args = NULL; + RlistAppendScalar(&args, path); + void *state = FileWatcherStateNew(args); + RlistDestroy(args); assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_ERROR); - DestroyFileWatcherState(state); + FileWatcherStateDestroy(state); LogSetGlobalLevel(old_level); TearDownSymlinkLoop(dir, loop); @@ -488,8 +500,13 @@ static void test_check_file_deleted_logs_error_once(void) const LogLevel old_level = LogGetGlobalLevel(); LogSetGlobalLevel(LOG_LEVEL_CRIT); - /* The failure at registration is logged */ - void *state = FileWatcherStateNew(path); + /* The failure of the first check is logged */ + Rlist *args = NULL; + RlistAppendScalar(&args, path); + void *state = FileWatcherStateNew(args); + RlistDestroy(args); + assert_int_equal(captured_err_count, 0); + assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_ERROR); assert_int_equal(captured_err_count, 1); assert_true(strstr(captured_err_message, "Unable to lstat") != NULL); @@ -507,7 +524,7 @@ static void test_check_file_deleted_logs_error_once(void) assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_ERROR); assert_int_equal(captured_err_count, 2); - DestroyFileWatcherState(state); + FileWatcherStateDestroy(state); LogSetGlobalLevel(old_level); LoggingPrivSetContext(NULL); @@ -524,8 +541,11 @@ static void test_event_watcher_check_failure_reported_once(void) const LogLevel old_level = LogGetGlobalLevel(); LogSetGlobalLevel(LOG_LEVEL_CRIT); + /* Registered while lstat() works, failing from then on */ WatcherRegistryInitialize(); - WatcherRegister(&expanded_promise, FileDeletedOptions(path), 1); + assert_int_equal(unlink(loop), 0); + assert_true(WatcherRegister(&expanded_promise, FileDeletedOptions(path, 1))); + assert_int_equal(symlink(loop, loop), 0); int fd = -1; assert_true(EventWatcherInitialize(&fd)); @@ -556,6 +576,32 @@ static void test_event_watcher_check_failure_reported_once(void) TearDownSymlinkLoop(dir, loop); } +static void test_watcher_register_check_failure(void) +{ + char dir[] = "/tmp/watcher_test.XXXXXX"; + char loop[PATH_MAX]; + char path[PATH_MAX]; + SetUpSymlinkLoop(dir, loop, sizeof(loop), path, sizeof(path)); + + const LogLevel old_level = LogGetGlobalLevel(); + LogSetGlobalLevel(LOG_LEVEL_CRIT); + + /* Without a reference, the watcher is not registered (its state is + * destroyed by the registry, checked by valgrind/ASan) */ + WatcherRegistryInitialize(); + assert_false(WatcherRegister(&expanded_promise, FileDeletedOptions(path, 1))); + + /* So the watcher thread doesn't check it, and doesn't report its failure */ + int fd = -1; + assert_true(EventWatcherInitialize(&fd)); + assert_false(WaitForQueuedEvent(fd, 20)); + + EventWatcherFinalize(); + + LogSetGlobalLevel(old_level); + TearDownSymlinkLoop(dir, loop); +} + int main() { PRINT_TEST_BANNER(); @@ -576,6 +622,7 @@ int main() unit_test(test_check_file_deleted_error), unit_test(test_check_file_deleted_logs_error_once), unit_test(test_event_watcher_check_failure_reported_once), + unit_test(test_watcher_register_check_failure), }; return run_tests(tests);