Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 4a22d95

Browse files
authored
Fix thread wait to be resilient to real time clock changes on Unix (#11213) (#12033)
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.
1 parent 1a93030 commit 4a22d95

File tree

5 files changed

+72
-18
lines changed

5 files changed

+72
-18
lines changed

src/pal/src/config.h.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@
9292
#cmakedefine01 HAVE_CLOCK_MONOTONIC_COARSE
9393
#cmakedefine01 HAVE_MACH_ABSOLUTE_TIME
9494
#cmakedefine01 HAVE_CLOCK_THREAD_CPUTIME
95+
#cmakedefine01 HAVE_PTHREAD_CONDATTR_SETCLOCK
9596
#cmakedefine01 STATVFS64_PROTOTYPE_BROKEN
9697
#cmakedefine01 HAVE_MMAP_DEV_ZERO
9798
#cmakedefine01 MMAP_IGNORES_HINT

src/pal/src/configure.cmake

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,9 @@ int main()
382382
383383
exit(ret);
384384
}" HAVE_CLOCK_MONOTONIC)
385+
386+
check_library_exists(pthread pthread_condattr_setclock "" HAVE_PTHREAD_CONDATTR_SETCLOCK)
387+
385388
check_cxx_source_runs("
386389
#include <stdlib.h>
387390
#include <time.h>

src/pal/src/synchmgr/synchmanager.cpp

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ namespace CorUnix
450450
if (dwTimeout != INFINITE)
451451
{
452452
// Calculate absolute timeout
453-
palErr = GetAbsoluteTimeout(dwTimeout, &tsAbsTmo);
453+
palErr = GetAbsoluteTimeout(dwTimeout, &tsAbsTmo, /*fPreferMonotonicClock*/ TRUE);
454454
if (NO_ERROR != palErr)
455455
{
456456
ERROR("Failed to convert timeout to absolute timeout\n");
@@ -1572,7 +1572,7 @@ namespace CorUnix
15721572
ptnwdWorkerThreadNativeData =
15731573
&pSynchManager->m_pthrWorker->synchronizationInfo.m_tnwdNativeData;
15741574

1575-
palErr = GetAbsoluteTimeout(WorkerThreadTerminationTimeout, &tsAbsTmo);
1575+
palErr = GetAbsoluteTimeout(WorkerThreadTerminationTimeout, &tsAbsTmo, /*fPreferMonotonicClock*/ TRUE);
15761576
if (NO_ERROR != palErr)
15771577
{
15781578
ERROR("Failed to convert timeout to absolute timeout\n");
@@ -4078,6 +4078,9 @@ namespace CorUnix
40784078
int iRet;
40794079
const int MaxUnavailableResourceRetries = 10;
40804080
int iEagains;
4081+
pthread_condattr_t attrs;
4082+
pthread_condattr_t *attrsPtr = nullptr;
4083+
40814084
m_shridWaitAwakened = RawSharedObjectAlloc(sizeof(DWORD),
40824085
DefaultSharedPool);
40834086
if (NULLSharedID == m_shridWaitAwakened)
@@ -4096,6 +4099,36 @@ namespace CorUnix
40964099
VolatileStore<DWORD>(pdwWaitState, TWS_ACTIVE);
40974100
m_tsThreadState = TS_STARTING;
40984101

4102+
#if HAVE_CLOCK_MONOTONIC && HAVE_PTHREAD_CONDATTR_SETCLOCK
4103+
attrsPtr = &attrs;
4104+
iRet = pthread_condattr_init(&attrs);
4105+
if (0 != iRet)
4106+
{
4107+
ERROR("Failed to initialize thread synchronization condition attribute "
4108+
"[error=%d (%s)]\n", iRet, strerror(iRet));
4109+
if (ENOMEM == iRet)
4110+
{
4111+
palErr = ERROR_NOT_ENOUGH_MEMORY;
4112+
}
4113+
else
4114+
{
4115+
palErr = ERROR_INTERNAL_ERROR;
4116+
}
4117+
goto IPrC_exit;
4118+
}
4119+
4120+
// Ensure that the pthread_cond_timedwait will use CLOCK_MONOTONIC
4121+
iRet = pthread_condattr_setclock(&attrs, CLOCK_MONOTONIC);
4122+
if (0 != iRet)
4123+
{
4124+
ERROR("Failed set thread synchronization condition timed wait clock "
4125+
"[error=%d (%s)]\n", iRet, strerror(iRet));
4126+
palErr = ERROR_INTERNAL_ERROR;
4127+
pthread_condattr_destroy(&attrs);
4128+
goto IPrC_exit;
4129+
}
4130+
#endif // HAVE_CLOCK_MONOTONIC && HAVE_PTHREAD_CONDATTR_SETCLOCK
4131+
40994132
iEagains = 0;
41004133
Mutex_retry:
41014134
iRet = pthread_mutex_init(&m_tnwdNativeData.mutex, NULL);
@@ -4121,7 +4154,9 @@ namespace CorUnix
41214154

41224155
iEagains = 0;
41234156
Cond_retry:
4124-
iRet = pthread_cond_init(&m_tnwdNativeData.cond, NULL);
4157+
4158+
iRet = pthread_cond_init(&m_tnwdNativeData.cond, attrsPtr);
4159+
41254160
if (0 != iRet)
41264161
{
41274162
ERROR("Failed creating thread synchronization condition "
@@ -4146,6 +4181,10 @@ namespace CorUnix
41464181
m_tnwdNativeData.fInitialized = true;
41474182

41484183
IPrC_exit:
4184+
if (attrsPtr != nullptr)
4185+
{
4186+
pthread_condattr_destroy(attrsPtr);
4187+
}
41494188
if (NO_ERROR != palErr)
41504189
{
41514190
m_tsThreadState = TS_FAILED;
@@ -4515,27 +4554,37 @@ namespace CorUnix
45154554
45164555
Converts a relative timeout to an absolute one.
45174556
--*/
4518-
PAL_ERROR CPalSynchronizationManager::GetAbsoluteTimeout(DWORD dwTimeout, struct timespec * ptsAbsTmo)
4557+
PAL_ERROR CPalSynchronizationManager::GetAbsoluteTimeout(DWORD dwTimeout, struct timespec * ptsAbsTmo, BOOL fPreferMonotonicClock)
45194558
{
45204559
PAL_ERROR palErr = NO_ERROR;
45214560
int iRet;
45224561

4523-
#if HAVE_WORKING_CLOCK_GETTIME
4524-
// Not every platform implements a (working) clock_gettime
4525-
iRet = clock_gettime(CLOCK_REALTIME, ptsAbsTmo);
4526-
#elif HAVE_WORKING_GETTIMEOFDAY
4527-
// Not every platform implements a (working) gettimeofday
4528-
struct timeval tv;
4529-
iRet = gettimeofday(&tv, NULL);
4530-
if (0 == iRet)
4562+
#if HAVE_CLOCK_MONOTONIC && HAVE_PTHREAD_CONDATTR_SETCLOCK
4563+
if (fPreferMonotonicClock)
45314564
{
4532-
ptsAbsTmo->tv_sec = tv.tv_sec;
4533-
ptsAbsTmo->tv_nsec = tv.tv_usec * tccMicroSecondsToNanoSeconds;
4565+
iRet = clock_gettime(CLOCK_MONOTONIC, ptsAbsTmo);
45344566
}
4567+
else
4568+
{
4569+
#endif
4570+
#if HAVE_WORKING_CLOCK_GETTIME
4571+
// Not every platform implements a (working) clock_gettime
4572+
iRet = clock_gettime(CLOCK_REALTIME, ptsAbsTmo);
4573+
#elif HAVE_WORKING_GETTIMEOFDAY
4574+
// Not every platform implements a (working) gettimeofday
4575+
struct timeval tv;
4576+
iRet = gettimeofday(&tv, NULL);
4577+
if (0 == iRet)
4578+
{
4579+
ptsAbsTmo->tv_sec = tv.tv_sec;
4580+
ptsAbsTmo->tv_nsec = tv.tv_usec * tccMicroSecondsToNanoSeconds;
4581+
}
45354582
#else
4536-
#error "Don't know how to get hi-res current time on this platform"
4583+
#error "Don't know how to get hi-res current time on this platform"
45374584
#endif // HAVE_WORKING_CLOCK_GETTIME, HAVE_WORKING_GETTIMEOFDAY
4538-
4585+
#if HAVE_CLOCK_MONOTONIC && HAVE_PTHREAD_CONDATTR_SETCLOCK
4586+
}
4587+
#endif
45394588
if (0 == iRet)
45404589
{
45414590
ptsAbsTmo->tv_sec += dwTimeout / tccSecondsToMillieSeconds;

src/pal/src/synchmgr/synchmanager.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1015,7 +1015,8 @@ namespace CorUnix
10151015

10161016
static PAL_ERROR GetAbsoluteTimeout(
10171017
DWORD dwTimeout,
1018-
struct timespec * ptsAbsTmo);
1018+
struct timespec * ptsAbsTmo,
1019+
BOOL fPreferMonotonicClock);
10191020
};
10201021
}
10211022

src/pal/src/synchobj/mutex.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,7 @@ MutexTryAcquireLockResult MutexHelpers::TryAcquireLock(pthread_mutex_t *mutex, D
858858
default:
859859
{
860860
struct timespec timeoutTime;
861-
PAL_ERROR palError = CPalSynchronizationManager::GetAbsoluteTimeout(timeoutMilliseconds, &timeoutTime);
861+
PAL_ERROR palError = CPalSynchronizationManager::GetAbsoluteTimeout(timeoutMilliseconds, &timeoutTime, /*fPreferMonotonicClock*/ FALSE);
862862
_ASSERTE(palError == NO_ERROR);
863863
lockResult = pthread_mutex_timedlock(mutex, &timeoutTime);
864864
break;

0 commit comments

Comments
 (0)