From 4a0d63625df171a72a79d2a775cc563a82e2b03e Mon Sep 17 00:00:00 2001 From: Maciej Wolny Date: Mon, 14 Jan 2019 21:18:31 +0000 Subject: [PATCH] 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 [1] https://sourceware.org/ml/libc-alpha/2018-12/msg00512.html v2: Check if CLOCK_MONOTONIC is available. Signed-off-by: Maciej Wolny --- common/wscript | 3 ++ posix/JackPosixCommon.cpp | 50 +++++++++++++++++++++ posix/JackPosixCommon.h | 30 +++++++++++++ posix/JackPosixSemaphore.cpp | 84 +++++++++++++++++++++++++++++------- 4 files changed, 151 insertions(+), 16 deletions(-) create mode 100644 posix/JackPosixCommon.cpp create mode 100644 posix/JackPosixCommon.h diff --git a/common/wscript b/common/wscript index 48c0ad9d..c89263f6 100644 --- a/common/wscript +++ b/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', diff --git a/posix/JackPosixCommon.cpp b/posix/JackPosixCommon.cpp new file mode 100644 index 00000000..97666bfc --- /dev/null +++ b/posix/JackPosixCommon.cpp @@ -0,0 +1,50 @@ +#include "JackPosixCommon.h" +#include "JackConstants.h" +#include "JackTools.h" +#include "JackError.h" +#include +#include +#include +#include +#include + +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 + + diff --git a/posix/JackPosixCommon.h b/posix/JackPosixCommon.h new file mode 100644 index 00000000..54c529a9 --- /dev/null +++ b/posix/JackPosixCommon.h @@ -0,0 +1,30 @@ +#ifndef __JackPosixCommon__ +#define __JackPosixCommon__ + +#include "jslist.h" +#include "JackCompilerDeps.h" +#include "JackError.h" + +#include +#include +#include + +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 + diff --git a/posix/JackPosixSemaphore.cpp b/posix/JackPosixSemaphore.cpp index 1f4df7a4..3d763a2e 100644 --- a/posix/JackPosixSemaphore.cpp +++ b/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 #include #include @@ -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