From: Jan Vorlicek Date: Wed, 26 Apr 2017 13:48:20 +0000 (-0700) Subject: Fix thread wait to be resilient to real time clock changes on Unix (dotnet/coreclr... X-Git-Tag: submit/tizen/20210909.063632~11030^2~7113 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f4a0ce1e0b724e185d392227d0ee4f24260a3fb4;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Fix thread wait to be resilient to real time clock changes on Unix (dotnet/coreclr#11213) 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. Commit migrated from https://github.com/dotnet/coreclr/commit/a5a8c6cdc6e3b732fc5a344878b165f149d99729 --- diff --git a/src/coreclr/src/pal/src/config.h.in b/src/coreclr/src/pal/src/config.h.in index e3024ac..7f37f42 100644 --- a/src/coreclr/src/pal/src/config.h.in +++ b/src/coreclr/src/pal/src/config.h.in @@ -103,6 +103,7 @@ #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 diff --git a/src/coreclr/src/pal/src/configure.cmake b/src/coreclr/src/pal/src/configure.cmake index f9a23e8..b5b98d5 100644 --- a/src/coreclr/src/pal/src/configure.cmake +++ b/src/coreclr/src/pal/src/configure.cmake @@ -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 #include diff --git a/src/coreclr/src/pal/src/synchmgr/synchmanager.cpp b/src/coreclr/src/pal/src/synchmgr/synchmanager.cpp index 3aec140..d836a17 100644 --- a/src/coreclr/src/pal/src/synchmgr/synchmanager.cpp +++ b/src/coreclr/src/pal/src/synchmgr/synchmanager.cpp @@ -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(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; diff --git a/src/coreclr/src/pal/src/synchmgr/synchmanager.hpp b/src/coreclr/src/pal/src/synchmgr/synchmanager.hpp index fdef82e..883d5b8 100644 --- a/src/coreclr/src/pal/src/synchmgr/synchmanager.hpp +++ b/src/coreclr/src/pal/src/synchmgr/synchmanager.hpp @@ -1015,7 +1015,8 @@ namespace CorUnix static PAL_ERROR GetAbsoluteTimeout( DWORD dwTimeout, - struct timespec * ptsAbsTmo); + struct timespec * ptsAbsTmo, + BOOL fPreferMonotonicClock); }; } diff --git a/src/coreclr/src/pal/src/synchobj/mutex.cpp b/src/coreclr/src/pal/src/synchobj/mutex.cpp index d929eaa..692f5e2 100644 --- a/src/coreclr/src/pal/src/synchobj/mutex.cpp +++ b/src/coreclr/src/pal/src/synchobj/mutex.cpp @@ -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;