From e91a91816496f60694e96dbd705f5b3ca49e33dc Mon Sep 17 00:00:00 2001 From: historic_bruno Date: Mon, 3 Dec 2012 13:24:12 +0000 Subject: [PATCH] Fixes race condition in EnsureMonotonic on 32-bit builds, which was causing unreliable timer behavior, fixes #1729 This was SVN commit r12927. --- source/lib/timer.cpp | 30 +++++-------------- source/tools/atlas/GameInterface/GameLoop.cpp | 6 ++-- 2 files changed, 10 insertions(+), 26 deletions(-) diff --git a/source/lib/timer.cpp b/source/lib/timer.cpp index 227d67fdab..7cdbb924dc 100644 --- a/source/lib/timer.cpp +++ b/source/lib/timer.cpp @@ -34,6 +34,7 @@ #include #include "lib/module_init.h" +#include "lib/posix/posix_pthread.h" #include "lib/posix/posix_time.h" # include "lib/sysdep/cpu.h" #if OS_WIN @@ -81,33 +82,16 @@ void timer_LatchStartTime() #endif } - +static pthread_mutex_t ensure_monotonic_mutex = PTHREAD_MUTEX_INITIALIZER; // NB: does not guarantee strict monotonicity - callers must avoid // dividing by the difference of two equal times. -// (this avoids having to update maxRepresentation when -// updating newTime to the (more recent) oldTime) static void EnsureMonotonic(double& newTime) { - i64 newRepresentation; - memcpy(&newRepresentation, &newTime, sizeof(newRepresentation)); - - // representation of the latest time reported by any thread - static volatile i64 maxRepresentation; // initially 0.0 - -retry: - const i64 oldRepresentation = maxRepresentation; // latch - double oldTime; - memcpy(&oldTime, &oldRepresentation, sizeof(oldTime)); - // a previous or concurrent call got a more recent time - - // return that and avoid updating maxRepresentation. - if(newTime < oldTime) - { - newTime = oldTime; - return; - } - - if(!cpu_CAS64(&maxRepresentation, oldRepresentation, newRepresentation)) - goto retry; + pthread_mutex_lock(&ensure_monotonic_mutex); + static double maxTime; + maxTime = std::max(maxTime, newTime); + newTime = maxTime; + pthread_mutex_unlock(&ensure_monotonic_mutex); } diff --git a/source/tools/atlas/GameInterface/GameLoop.cpp b/source/tools/atlas/GameInterface/GameLoop.cpp index 484a4d2853..1c4d44f791 100644 --- a/source/tools/atlas/GameInterface/GameLoop.cpp +++ b/source/tools/atlas/GameInterface/GameLoop.cpp @@ -142,11 +142,11 @@ static void* RunEngine(void* data) // Calculate frame length { - double time = timer_Time(); + const double time = timer_Time(); static double last_time = time; - // TODO: why is this sometimes negative, if timer_Time ensures monotonic results? - float realFrameLength = std::max(0.0f, (float)(time-last_time)); + const double realFrameLength = time-last_time; last_time = time; + ENSURE(realFrameLength >= 0.0); // TODO: filter out big jumps, e.g. when having done a lot of slow // processing in the last frame state.realFrameLength = realFrameLength;