Author: pjd Date: Mon Aug 30 00:06:05 2010 New Revision: 211982 URL: http://svn.freebsd.org/changeset/base/211982
Log: Use sigtimedwait(2) for signals handling in primary process. This fixes various races and eliminates use of pthread* API in signal handler. Pointed out by: kib With help from: jilles MFC after: 2 weeks Obtained from: Wheel Systems Sp. z o.o. http://www.wheelsystems.com Modified: head/sbin/hastd/primary.c Modified: head/sbin/hastd/primary.c ============================================================================== --- head/sbin/hastd/primary.c Sun Aug 29 22:55:21 2010 (r211981) +++ head/sbin/hastd/primary.c Mon Aug 30 00:06:05 2010 (r211982) @@ -46,6 +46,7 @@ __FBSDID("$FreeBSD$"); #include <fcntl.h> #include <libgeom.h> #include <pthread.h> +#include <signal.h> #include <stdint.h> #include <stdio.h> #include <string.h> @@ -132,8 +133,6 @@ static pthread_cond_t sync_cond; * The lock below allows to synchornize access to remote connections. */ static pthread_rwlock_t *hio_remote_lock; -static pthread_mutex_t hio_guard_lock; -static pthread_cond_t hio_guard_cond; /* * Lock to synchronize metadata updates. Also synchronize access to @@ -152,13 +151,9 @@ static pthread_mutex_t metadata_lock; */ #define HAST_NCOMPONENTS 2 /* - * Number of seconds to sleep between keepalive packets. + * Number of seconds to sleep between reconnect retries or keepalive packets. */ -#define KEEPALIVE_SLEEP 10 -/* - * Number of seconds to sleep between reconnect retries. - */ -#define RECONNECT_SLEEP 5 +#define RETRY_SLEEP 10 #define ISCONNECTED(res, no) \ ((res)->hr_remotein != NULL && (res)->hr_remoteout != NULL) @@ -230,7 +225,11 @@ static void *ggate_send_thread(void *arg static void *sync_thread(void *arg); static void *guard_thread(void *arg); -static void sighandler(int sig); +static void +dummy_sighandler(int sig __unused) +{ + /* Nothing to do. */ +} static void cleanup(struct hast_resource *res) @@ -319,6 +318,7 @@ init_environment(struct hast_resource *r { struct hio *hio; unsigned int ii, ncomps; + sigset_t mask; /* * In the future it might be per-resource value. @@ -389,8 +389,6 @@ init_environment(struct hast_resource *r TAILQ_INIT(&hio_done_list); mtx_init(&hio_done_list_lock); cv_init(&hio_done_list_cond); - mtx_init(&hio_guard_lock); - cv_init(&hio_guard_cond); mtx_init(&metadata_lock); /* @@ -431,10 +429,10 @@ init_environment(struct hast_resource *r /* * Turn on signals handling. */ - signal(SIGINT, sighandler); - signal(SIGTERM, sighandler); - signal(SIGHUP, sighandler); - signal(SIGCHLD, sighandler); + /* Because SIGCHLD is ignored by default, setup dummy handler for it. */ + PJDLOG_VERIFY(signal(SIGCHLD, dummy_sighandler) != SIG_ERR); + PJDLOG_VERIFY(sigfillset(&mask) == 0); + PJDLOG_VERIFY(sigprocmask(SIG_SETMASK, &mask, NULL) == 0); } static void @@ -893,16 +891,6 @@ remote_close(struct hast_resource *res, * Stop synchronization if in-progress. */ sync_stop(); - - /* - * Wake up guard thread (if we are not called from within guard thread), - * so it can immediately start reconnect. - */ - if (!mtx_owned(&hio_guard_lock)) { - mtx_lock(&hio_guard_lock); - cv_signal(&hio_guard_cond); - mtx_unlock(&hio_guard_lock); - } } /* @@ -1734,35 +1722,6 @@ free_queue: } static void -sighandler(int sig) -{ - bool unlock; - - switch (sig) { - case SIGINT: - case SIGTERM: - sigexit_received = true; - break; - case SIGHUP: - sighup_received = true; - break; - case SIGCHLD: - sigchld_received = true; - break; - default: - assert(!"invalid condition"); - } - /* - * Racy, but if we cannot obtain hio_guard_lock here, we don't - * want to risk deadlock. - */ - unlock = mtx_trylock(&hio_guard_lock); - cv_signal(&hio_guard_cond); - if (unlock) - mtx_unlock(&hio_guard_lock); -} - -static void config_reload(void) { struct hastd_config *newcfg; @@ -1973,48 +1932,48 @@ guard_thread(void *arg) { struct hast_resource *res = arg; unsigned int ii, ncomps; + struct timespec timeout; time_t lastcheck, now; - int timeout; + sigset_t mask; + int signo; ncomps = HAST_NCOMPONENTS; lastcheck = time(NULL); + PJDLOG_VERIFY(sigemptyset(&mask) == 0); + PJDLOG_VERIFY(sigaddset(&mask, SIGHUP) == 0); + PJDLOG_VERIFY(sigaddset(&mask, SIGINT) == 0); + PJDLOG_VERIFY(sigaddset(&mask, SIGTERM) == 0); + PJDLOG_VERIFY(sigaddset(&mask, SIGCHLD) == 0); + + timeout.tv_nsec = 0; + signo = -1; + for (;;) { - if (sigexit_received) { + switch (signo) { + case SIGHUP: + config_reload(); + break; + case SIGINT: + case SIGTERM: + sigexit_received = true; primary_exitx(EX_OK, "Termination signal received, exiting."); + break; + default: + break; } - if (sighup_received) { - sighup_received = false; - config_reload(); - } - hook_check(sigchld_received); - if (sigchld_received) - sigchld_received = false; + hook_check(signo == SIGCHLD); pjdlog_debug(2, "remote_guard: Checking connections."); - mtx_lock(&hio_guard_lock); - timeout = KEEPALIVE_SLEEP; - for (ii = 0; ii < ncomps; ii++) { - if (!ISCONNECTED(res, ii)) { - timeout = RECONNECT_SLEEP; - break; - } - } now = time(NULL); - if (lastcheck + timeout <= now) { - timeout = KEEPALIVE_SLEEP; - for (ii = 0; ii < ncomps; ii++) { + if (lastcheck + RETRY_SLEEP <= now) { + for (ii = 0; ii < ncomps; ii++) guard_one(res, ii); - if (!ISCONNECTED(res, ii)) - timeout = RECONNECT_SLEEP; - } lastcheck = now; } - /* Sleep only if a signal wasn't delivered in the meantime. */ - if (!sigexit_received && !sighup_received && !sigchld_received) - cv_timedwait(&hio_guard_cond, &hio_guard_lock, timeout); - mtx_unlock(&hio_guard_lock); + timeout.tv_sec = RETRY_SLEEP; + signo = sigtimedwait(&mask, NULL, &timeout); } /* NOTREACHED */ return (NULL); _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
