From dc81071d9fe21beaead51a9c0a8d87fbefbfcf56 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@nicira.com>
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 225827ef5..2cb4f345a 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 7df8a654f..84abdfae4 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 7da3b383b..5ba903e3a 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.43.0