From dc81071d9fe21beaead51a9c0a8d87fbefbfcf56 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 15 Oct 2009 10:42:59 -0700 Subject: [PATCH] cfg: Fix implementation of timeout in attempting to lock the config file. Without removing SA_RESTART from the SIGALRM handler, the fcntl call will never return, even after the signal handler is invoked and returns. We haven't seen a problem in practice, at least not recently, but that's probably just luck combined with not holding the configuration file lock for very long. --- lib/cfg.c | 9 ++++++++- lib/timeval.c | 42 +++++++++++++++++++++++++++++++++++++----- lib/timeval.h | 2 ++ 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/lib/cfg.c b/lib/cfg.c index 225827ef..2cb4f345 100644 --- a/lib/cfg.c +++ b/lib/cfg.c @@ -320,12 +320,19 @@ static int try_lock(int fd, bool block) { struct flock l; + int error; + memset(&l, 0, sizeof l); l.l_type = F_WRLCK; l.l_whence = SEEK_SET; l.l_start = 0; l.l_len = 0; - return fcntl(fd, block ? F_SETLKW : F_SETLK, &l) == -1 ? errno : 0; + + time_disable_restart(); + error = fcntl(fd, block ? F_SETLKW : F_SETLK, &l) == -1 ? errno : 0; + time_enable_restart(); + + return error; } /* Locks the configuration file against modification by other processes and diff --git a/lib/timeval.c b/lib/timeval.c index 7df8a654..84abdfae 100644 --- a/lib/timeval.c +++ b/lib/timeval.c @@ -44,6 +44,7 @@ static struct timeval now; static time_t deadline = TIME_MIN; static void setup_timer(void); +static void setup_signal(int flags); static void sigalrm_handler(int); static void refresh_if_ticked(void); static time_t time_add(time_t, time_t); @@ -56,7 +57,6 @@ static void log_poll_interval(long long int last_wakeup, void time_init(void) { - struct sigaction sa; if (inited) { return; } @@ -67,17 +67,49 @@ time_init(void) gettimeofday(&now, NULL); tick = false; - /* Set up signal handler. */ + setup_signal(SA_RESTART); + setup_timer(); +} + +static void +setup_signal(int flags) +{ + struct sigaction sa; + memset(&sa, 0, sizeof sa); sa.sa_handler = sigalrm_handler; sigemptyset(&sa.sa_mask); - sa.sa_flags = SA_RESTART; + sa.sa_flags = flags; if (sigaction(SIGALRM, &sa, NULL)) { ovs_fatal(errno, "sigaction(SIGALRM) failed"); } +} - /* Set up periodic signal. */ - setup_timer(); +/* Remove SA_RESTART from the flags for SIGALRM, so that any system call that + * is interrupted by the periodic timer interrupt will return EINTR instead of + * continuing after the signal handler returns. + * + * time_disable_restart() and time_enable_restart() may be usefully wrapped + * around function calls that might otherwise block forever unless interrupted + * by a signal, e.g.: + * + * time_disable_restart(); + * fcntl(fd, F_SETLKW, &lock); + * time_enable_restart(); + */ +void +time_disable_restart(void) +{ + setup_signal(0); +} + +/* Add SA_RESTART to the flags for SIGALRM, so that any system call that + * is interrupted by the periodic timer interrupt will continue after the + * signal handler returns instead of returning EINTR. */ +void +time_enable_restart(void) +{ + setup_signal(SA_RESTART); } static void diff --git a/lib/timeval.h b/lib/timeval.h index 7da3b383..5ba903e3 100644 --- a/lib/timeval.h +++ b/lib/timeval.h @@ -41,6 +41,8 @@ BUILD_ASSERT_DECL(TYPE_IS_SIGNED(time_t)); #define TIME_UPDATE_INTERVAL 100 void time_init(void); +void time_disable_restart(void); +void time_enable_restart(void); void time_postfork(void); void time_refresh(void); time_t time_now(void); -- 2.30.2