Fix thread wait to be resilient to real time clock changes on Unix (#11213)
authorJan Vorlicek <janvorli@microsoft.com>
Wed, 26 Apr 2017 13:48:20 +0000 (06:48 -0700)
committerGitHub <noreply@github.com>
Wed, 26 Apr 2017 13:48:20 +0000 (06:48 -0700)
This change fixes a problem that happens when a thread uses a timed wait
and the real time clock change in the system (e.g. due to the user action,
time server sync, leap second, etc). In such case, the wait may end up waiting
for shorter or longer period of time.
It fixes the issue for Thread.Sleep and timed wait on a process local synchronization
primitives by using CLOCK_MONOTONIC instead of CLOCK_REALTIME for waiting on
cond vars.
Unfortunately, the waits on cross process synchronication primitives like
named mutexes always use CLOCK_REALTIME, so this fix works on process local primitives
only.

src/pal/src/config.h.in
src/pal/src/configure.cmake
src/pal/src/synchmgr/synchmanager.cpp
src/pal/src/synchmgr/synchmanager.hpp
src/pal/src/synchobj/mutex.cpp

index e3024ac..7f37f42 100644 (file)
 #cmakedefine01 HAVE_CLOCK_MONOTONIC_COARSE
 #cmakedefine01 HAVE_MACH_ABSOLUTE_TIME
 #cmakedefine01 HAVE_CLOCK_THREAD_CPUTIME
+#cmakedefine01 HAVE_PTHREAD_CONDATTR_SETCLOCK
 #cmakedefine01 STATVFS64_PROTOTYPE_BROKEN
 #cmakedefine01 HAVE_MMAP_DEV_ZERO
 #cmakedefine01 MMAP_ANON_IGNORES_PROTECTION
index f9a23e8..b5b98d5 100644 (file)
@@ -417,6 +417,9 @@ int main()
 
   exit(ret);
 }" HAVE_CLOCK_MONOTONIC)
+
+check_library_exists(pthread pthread_condattr_setclock "" HAVE_PTHREAD_CONDATTR_SETCLOCK)
+
 check_cxx_source_runs("
 #include <stdlib.h>
 #include <time.h>
index 3aec140..d836a17 100644 (file)
@@ -450,7 +450,7 @@ namespace CorUnix
         if (dwTimeout != INFINITE)
         {
             // Calculate absolute timeout
-            palErr = GetAbsoluteTimeout(dwTimeout, &tsAbsTmo);
+            palErr = GetAbsoluteTimeout(dwTimeout, &tsAbsTmo, /*fPreferMonotonicClock*/ TRUE);
             if (NO_ERROR != palErr)
             {
                 ERROR("Failed to convert timeout to absolute timeout\n");
@@ -1572,7 +1572,7 @@ namespace CorUnix
         ptnwdWorkerThreadNativeData =
             &pSynchManager->m_pthrWorker->synchronizationInfo.m_tnwdNativeData;
 
-        palErr = GetAbsoluteTimeout(WorkerThreadTerminationTimeout, &tsAbsTmo);
+        palErr = GetAbsoluteTimeout(WorkerThreadTerminationTimeout, &tsAbsTmo, /*fPreferMonotonicClock*/ TRUE);
         if (NO_ERROR != palErr)
         {
             ERROR("Failed to convert timeout to absolute timeout\n");
@@ -4078,6 +4078,9 @@ namespace CorUnix
         int iRet;
         const int MaxUnavailableResourceRetries = 10;
         int iEagains;
+        pthread_condattr_t attrs;
+        pthread_condattr_t *attrsPtr = nullptr;
+
         m_shridWaitAwakened = RawSharedObjectAlloc(sizeof(DWORD),
                                                    DefaultSharedPool);
         if (NULLSharedID == m_shridWaitAwakened)
@@ -4096,6 +4099,36 @@ namespace CorUnix
         VolatileStore<DWORD>(pdwWaitState, TWS_ACTIVE);
         m_tsThreadState = TS_STARTING;
 
+#if HAVE_CLOCK_MONOTONIC && HAVE_PTHREAD_CONDATTR_SETCLOCK
+        attrsPtr = &attrs;
+        iRet = pthread_condattr_init(&attrs);
+        if (0 != iRet)
+        {
+            ERROR("Failed to initialize thread synchronization condition attribute "
+                  "[error=%d (%s)]\n", iRet, strerror(iRet));
+            if (ENOMEM == iRet)
+            {
+                palErr = ERROR_NOT_ENOUGH_MEMORY;
+            }
+            else
+            {
+                palErr = ERROR_INTERNAL_ERROR;
+            }
+            goto IPrC_exit;
+        }
+
+        // Ensure that the pthread_cond_timedwait will use CLOCK_MONOTONIC
+        iRet = pthread_condattr_setclock(&attrs, CLOCK_MONOTONIC);
+        if (0 != iRet)
+        {
+            ERROR("Failed set thread synchronization condition timed wait clock "
+                  "[error=%d (%s)]\n", iRet, strerror(iRet));
+            palErr = ERROR_INTERNAL_ERROR;
+            pthread_condattr_destroy(&attrs);
+            goto IPrC_exit;
+        }
+#endif // HAVE_CLOCK_MONOTONIC && HAVE_PTHREAD_CONDATTR_SETCLOCK
+
         iEagains = 0;
     Mutex_retry:
         iRet = pthread_mutex_init(&m_tnwdNativeData.mutex, NULL);
@@ -4121,7 +4154,9 @@ namespace CorUnix
 
         iEagains = 0;
     Cond_retry:
-        iRet = pthread_cond_init(&m_tnwdNativeData.cond, NULL);
+
+        iRet = pthread_cond_init(&m_tnwdNativeData.cond, attrsPtr);
+
         if (0 != iRet)
         {
             ERROR("Failed creating thread synchronization condition "
@@ -4146,6 +4181,10 @@ namespace CorUnix
         m_tnwdNativeData.fInitialized = true;
 
     IPrC_exit:
+        if (attrsPtr != nullptr)
+        {
+            pthread_condattr_destroy(attrsPtr);
+        }
         if (NO_ERROR != palErr)
         {
             m_tsThreadState = TS_FAILED;
@@ -4515,27 +4554,37 @@ namespace CorUnix
 
     Converts a relative timeout to an absolute one.
     --*/
-    PAL_ERROR CPalSynchronizationManager::GetAbsoluteTimeout(DWORD dwTimeout, struct timespec * ptsAbsTmo)
+    PAL_ERROR CPalSynchronizationManager::GetAbsoluteTimeout(DWORD dwTimeout, struct timespec * ptsAbsTmo, BOOL fPreferMonotonicClock)
     {
         PAL_ERROR palErr = NO_ERROR;
         int iRet;
 
-#if HAVE_WORKING_CLOCK_GETTIME
-        // Not every platform implements a (working) clock_gettime
-        iRet = clock_gettime(CLOCK_REALTIME, ptsAbsTmo);
-#elif HAVE_WORKING_GETTIMEOFDAY
-        // Not every platform implements a (working) gettimeofday
-        struct timeval tv;
-        iRet = gettimeofday(&tv, NULL);
-        if (0 == iRet)
+#if HAVE_CLOCK_MONOTONIC && HAVE_PTHREAD_CONDATTR_SETCLOCK
+        if (fPreferMonotonicClock)
         {
-            ptsAbsTmo->tv_sec  = tv.tv_sec;
-            ptsAbsTmo->tv_nsec = tv.tv_usec * tccMicroSecondsToNanoSeconds;
+            iRet = clock_gettime(CLOCK_MONOTONIC, ptsAbsTmo);
         }
+        else
+        {
+#endif        
+#if HAVE_WORKING_CLOCK_GETTIME
+            // Not every platform implements a (working) clock_gettime
+            iRet = clock_gettime(CLOCK_REALTIME, ptsAbsTmo);
+#elif HAVE_WORKING_GETTIMEOFDAY
+            // Not every platform implements a (working) gettimeofday
+            struct timeval tv;
+            iRet = gettimeofday(&tv, NULL);
+            if (0 == iRet)
+            {
+                ptsAbsTmo->tv_sec  = tv.tv_sec;
+                ptsAbsTmo->tv_nsec = tv.tv_usec * tccMicroSecondsToNanoSeconds;
+            }
 #else
-        #error "Don't know how to get hi-res current time on this platform"
+            #error "Don't know how to get hi-res current time on this platform"
 #endif // HAVE_WORKING_CLOCK_GETTIME, HAVE_WORKING_GETTIMEOFDAY
-
+#if HAVE_CLOCK_MONOTONIC && HAVE_PTHREAD_CONDATTR_SETCLOCK
+        }
+#endif
         if (0 == iRet)
         {
             ptsAbsTmo->tv_sec  += dwTimeout / tccSecondsToMillieSeconds;
index fdef82e..883d5b8 100644 (file)
@@ -1015,7 +1015,8 @@ namespace CorUnix
 
         static PAL_ERROR GetAbsoluteTimeout(
             DWORD dwTimeout,
-            struct timespec * ptsAbsTmo);
+            struct timespec * ptsAbsTmo,
+            BOOL fPreferMonotonicClock);
     };
 }
 
index d929eaa..692f5e2 100644 (file)
@@ -859,7 +859,7 @@ MutexTryAcquireLockResult MutexHelpers::TryAcquireLock(pthread_mutex_t *mutex, D
         default:
         {
             struct timespec timeoutTime;
-            PAL_ERROR palError = CPalSynchronizationManager::GetAbsoluteTimeout(timeoutMilliseconds, &timeoutTime);
+            PAL_ERROR palError = CPalSynchronizationManager::GetAbsoluteTimeout(timeoutMilliseconds, &timeoutTime, /*fPreferMonotonicClock*/ FALSE);
             _ASSERTE(palError == NO_ERROR);
             lockResult = pthread_mutex_timedlock(mutex, &timeoutTime);
             break;