summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNat Goodspeed <nat@lindenlab.com>2023-11-14 20:29:51 -0500
committerNat Goodspeed <nat@lindenlab.com>2023-11-14 20:29:51 -0500
commitd255c3dda852731b6709ac4e9c9821b3be84ec86 (patch)
treef980cc66f2689564ab2f41ba5e44fe9eaacdcaab
parent6654ad14eed674e894d2903e0f2ea37c4e806c0f (diff)
DRTVWR-588: Try to fix sporadic llrand test failures.
With GitHub viewer builds, every few weeks we've seen test failures when ll_frand() returns exactly 1.0. This is a problem for a function that's supposed to return [0.0 .. 1.0). Monty suggests that the problem is likely to be conversion of F32 to F64 to pass to fmod(), and then truncation of fmod()'s F64 result back to F32. Moved the clamping code to each size-specific ll_internal_random specialization. Monty also noted that a stateful static random number engine isn't thread-safe. Added a mutex lock.
-rw-r--r--indra/llcommon/llrand.cpp20
1 files changed, 18 insertions, 2 deletions
diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp
index 33afc50cf7..20e25177f0 100644
--- a/indra/llcommon/llrand.cpp
+++ b/indra/llcommon/llrand.cpp
@@ -28,6 +28,7 @@
#include "llrand.h"
#include "lluuid.h"
+#include "mutex.h"
/**
* Through analysis, we have decided that we want to take values which
@@ -58,8 +59,17 @@
* to restore uniform distribution.
*/
+static std::mutex gRandomGeneratorMutex;
static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed());
+inline F64 ll_internal_random_unclamped()
+{
+ // gRandomGenerator is a stateful static object, which is therefore not
+ // inherently thread-safe. Lock it before use.
+ std::unique_lock lk(gRandomGeneratorMutex);
+ return gRandomGenerator();
+}
+
// no default implementation, only specific F64 and F32 specializations
template <typename REAL>
inline REAL ll_internal_random();
@@ -71,7 +81,7 @@ inline F64 ll_internal_random<F64>()
// CPUs (or at least multi-threaded processes) seem to
// occasionally give an obviously incorrect random number -- like
// 5^15 or something. Sooooo, clamp it as described above.
- F64 rv = gRandomGenerator();
+ F64 rv{ ll_internal_random_unclamped() };
if(!((rv >= 0.0) && (rv < 1.0))) return fmod(rv, 1.0);
return rv;
}
@@ -79,7 +89,13 @@ inline F64 ll_internal_random<F64>()
template <>
inline F32 ll_internal_random<F32>()
{
- return F32(ll_internal_random<F64>());
+ // *HACK: clamp the result as described above.
+ // Per Monty, it's important to clamp using the correct fmodf() rather
+ // than expanding to F64 for fmod() and then truncating back to F32. Prior
+ // to this change, we were getting sporadic ll_frand() == 1.0 results.
+ F32 rv{ ll_internal_random_unclamped() };
+ if(!((rv >= 0.0) && (rv < 1.0))) return fmodf(rv, 1.0f);
+ return rv;
}
/*------------------------------ F64 aliases -------------------------------*/