posix: Wrap sem_timedwait() to check for time jumps

POSIX functions with names ending with _timedwait use the real time
clock and as such are sensitive to changes in system time during the
call. This is described in: [1].

This patch implements a wrapper for sem_timedwait() that, if the
function call fails, will check if the failure is due to a time warp,
and if so, will restart the call.

Based on idea and implementation by Ben Hutchings <ben.hutchings@codethink.co.uk>

[1] https://sourceware.org/ml/libc-alpha/2018-12/msg00512.html

v2: Check if CLOCK_MONOTONIC is available.

Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk>

common/wscript

@@ -81,6 +81,7 @@ def build(bld):
             'JackDebugClient.cpp',
             'timestamps.c',
             'promiscuous.c',
+            '../posix/JackPosixCommon.cpp',
             '../posix/JackPosixThread.cpp',
             '../posix/JackPosixProcessSync.cpp',
             '../posix/JackPosixMutex.cpp',
@@ -97,6 +98,7 @@ def build(bld):
             'JackDebugClient.cpp',
             'timestamps.c',
             'promiscuous.c',
+           '../posix/JackPosixCommon.cpp',
            '../posix/JackPosixThread.cpp',
            '../posix/JackFifo.cpp',
            '../posix/JackPosixProcessSync.cpp',
@@ -112,6 +114,7 @@ def build(bld):
             'JackDebugClient.cpp',
             'timestamps.c',
             'promiscuous.c',
+            '../posix/JackPosixCommon.cpp',
             '../posix/JackPosixProcessSync.cpp',
             '../posix/JackPosixThread.cpp',
             '../posix/JackPosixMutex.cpp',

posix/JackPosixCommon.cpp

@@ -0,0 +1,50 @@
+#include "JackPosixCommon.h"
+#include "JackConstants.h"
+#include "JackTools.h"
+#include "JackError.h"
+#include <assert.h>
+#include <time.h>
+#include <string.h>
+#include <stdio.h>
+#include <unistd.h>
+
+namespace Jack
+{
+
+void JackPosixTools::TimespecAdd(const struct timespec *left,
+    const struct timespec *right, struct timespec *result)
+{
+	assert(result != NULL);
+    result->tv_sec = left->tv_sec + right->tv_sec;
+    result->tv_nsec = left->tv_nsec + right->tv_nsec;
+    if (result->tv_nsec >= 1000000000) {
+        result->tv_nsec -= 1000000000;
+        result->tv_sec += 1;
+    }
+}
+
+void JackPosixTools::TimespecSub(const struct timespec *left,
+    const struct timespec *right, struct timespec *result)
+{
+    assert(result != NULL);
+    result->tv_sec = left->tv_sec - right->tv_sec;
+    result->tv_nsec = left->tv_nsec - right->tv_nsec;
+    if (result->tv_nsec < 0) {
+        result->tv_nsec += 1000000000;
+        result->tv_sec -= 1;
+    }
+}
+
+int JackPosixTools::TimespecCmp(const struct timespec *left,
+    const struct timespec *right)
+{
+    if (left->tv_sec != right->tv_sec)
+        return left->tv_sec < right->tv_sec ? -1 : 1;
+    if (left->tv_nsec != right->tv_nsec)
+        return left->tv_nsec < right->tv_nsec ? -1 : 1;
+    return 0;
+}
+
+} // end of namespace
+
+

posix/JackPosixCommon.h

@@ -0,0 +1,30 @@
+#ifndef __JackPosixCommon__
+#define __JackPosixCommon__
+
+#include "jslist.h"
+#include "JackCompilerDeps.h"
+#include "JackError.h"
+
+#include <errno.h>
+#include <time.h>
+#include <unistd.h>
+
+namespace Jack
+{
+
+struct SERVER_EXPORT JackPosixTools
+{
+    static void TimespecAdd(const struct timespec *left,
+        const struct timespec *right, struct timespec *result);
+
+    static void TimespecSub(const struct timespec *left,
+        const struct timespec *right, struct timespec *result);
+
+    static int TimespecCmp(const struct timespec *left,
+        const struct timespec *right);
+};
+
+} // end of namespace
+
+#endif
+

posix/JackPosixSemaphore.cpp

@@ -21,6 +21,7 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
 #include "JackTools.h"
 #include "JackConstants.h"
 #include "JackError.h"
+#include "JackPosixCommon.h"
 #include <fcntl.h>
 #include <stdio.h>
 #include <sys/time.h>
@@ -115,29 +116,80 @@ bool JackPosixSemaphore::Wait()
 
 bool JackPosixSemaphore::TimedWait(long usec)
 {
-	int res;
-	struct timeval now;
-	timespec time;
+    int res;
+    struct timespec rel_timeout, now_mono, end_mono, now_real, end_real;
+    struct timespec diff_mono;
 
 	if (!fSemaphore) {
 		jack_error("JackPosixSemaphore::TimedWait name = %s already deallocated!!", fName);
 		return false;
 	}
-	gettimeofday(&now, 0);
-	time.tv_sec = now.tv_sec + usec / 1000000;
-    long tv_usec = (now.tv_usec + (usec % 1000000));
-    time.tv_sec += tv_usec / 1000000;
-    time.tv_nsec = (tv_usec % 1000000) * 1000;
-
-    while ((res = sem_timedwait(fSemaphore, &time)) < 0) {
-        jack_error("JackPosixSemaphore::TimedWait err = %s", strerror(errno));
-        jack_log("JackPosixSemaphore::TimedWait now : %ld %ld ", now.tv_sec, now.tv_usec);
-        jack_log("JackPosixSemaphore::TimedWait next : %ld %ld ", time.tv_sec, time.tv_nsec/1000);
-        if (errno != EINTR) {
-            break;
+
+    /* Convert usec argument to timespec */
+    rel_timeout.tv_sec = usec / 1000000;
+    rel_timeout.tv_nsec = (usec % 1000000) * 1000;
+
+    /* Calculate absolute monotonic timeout */
+    res = clock_gettime(CLOCK_MONOTONIC, &now_mono);
+    if (res != 0) {
+        /* CLOCK_MONOTONIC is not supported, do not check for time skips. */
+        res = clock_gettime(CLOCK_REALTIME, &now_real);
+        assert(res == 0);
+        JackPosixTools::TimespecAdd(&now_real, &rel_timeout, &end_real);
+        while ((res = sem_timedwait(fSemaphore, &end_real)) < 0) {
+            if (errno != EINTR) {
+                break;
+            }
+        }
+        if (res == 0) {
+            return true;
+        } else {
+            goto err;
         }
     }
-    return (res == 0);
+    JackPosixTools::TimespecAdd(&now_mono, &rel_timeout, &end_mono);
+
+    /* sem_timedwait() is affected by abrupt time jumps, i.e. when the system
+     * time is changed. To protect against this, measure the time difference
+     * between and after the sem_timedwait() call and if it suggests that there
+     * has been a time jump, restart the call. */
+    for (;;) {
+        /* Calculate absolute realtime timeout, assuming no steps */
+        res = clock_gettime(CLOCK_REALTIME, &now_real);
+        assert(res == 0);
+        JackPosixTools::TimespecSub(&end_mono, &now_mono, &diff_mono);
+        JackPosixTools::TimespecAdd(&now_real, &diff_mono, &end_real);
+
+        while ((res = sem_timedwait(fSemaphore, &end_real)) < 0) {
+            if (errno != EINTR) {
+                break;
+            }
+        }
+        if (res == 0) {
+            return true;
+        }
+        if (errno != ETIMEDOUT) {
+            goto err;
+        }
+
+        /* Compare with monotonic timeout, in case a step happened */
+        int old_errno = errno;
+        res = clock_gettime(CLOCK_MONOTONIC, &now_mono);
+        assert(res == 0);
+        errno = old_errno;
+        if (JackPosixTools::TimespecCmp(&now_mono, &end_mono) >= 0) {
+            goto err;
+        }
+    }
+    return true;
+
+err:
+    jack_error("JackPosixSemaphore::TimedWait err = %s", strerror(errno));
+    jack_log("JackPosixSemaphore::TimedWait now : %ld %ld ", now_real.tv_sec,
+        now_real.tv_nsec / 1000);
+    jack_log("JackPosixSemaphore::TimedWait next : %ld %ld ", end_real.tv_sec,
+        end_real.tv_nsec / 1000);
+    return false;
 }
 
 #else