[MAINT][code] Removed use of pthread_once. Fixed Haivision#3115

Author: Mikolaj Malecki

CMakeLists.txt

@@ -26,6 +26,7 @@ include(FindPThreadGetSetName)
 include(CheckGCCAtomicIntrinsics)
 include(CheckCXXAtomic)
 include(CheckCXXStdPutTime)
+include(CheckSourceCompiles)
 
 # This is required in some projects that add some other sources
 # to the SRT library to be compiled together (aka "virtual library").
@@ -490,7 +491,6 @@ else()
 endif()
 message(STATUS "WARNING OPTIONS: ${SRT_GCC_WARN}")
 
-
 # --------------------------------------------
 # Post-option variable synchronization
 # and option-to-preprocessor-macro transition
@@ -1174,6 +1174,22 @@ endmacro()
 srt_set_stdcxx(srt_virtual "${USE_CXX_STD_LIB}")
 srt_set_stdc(srt_virtual "99")
 
+check_source_compiles(CXX
+"#include <stdlib.h>
+int main(){
+  int randsr = 31337;
+  int v = rand_r(&randr);
+  return 0;
+}"
+HAVE_RAND_R
+)
+if (HAVE_RAND_R)
+	target_compile_definitions(srt_virtual PRIVATE SRT_HAVE_RAND_R=1)
+else()
+	target_compile_definitions(srt_virtual PRIVATE SRT_HAVE_RAND_R=0)
+endif()
+
+
 if (ENABLE_ENCRYPTION)
 	target_include_directories(srt_virtual PRIVATE  ${SSL_INCLUDE_DIRS})
 

srtcore/core.cpp

@@ -233,33 +233,12 @@ struct SrtOptionAction
 
 const SrtOptionAction s_sockopt_action;
 
-#if HAVE_CXX11
-
 CUDTUnited& CUDT::uglobal()
 {
     static CUDTUnited instance;
     return instance;
 }
 
-#else // !HAVE_CXX11
-
-static pthread_once_t s_UDTUnitedOnce = PTHREAD_ONCE_INIT;
-
-static CUDTUnited *getInstance()
-{
-    static CUDTUnited instance;
-    return &instance;
-}
-
-CUDTUnited& CUDT::uglobal()
-{
-    // We don't want lock each time, pthread_once can be faster than mutex.
-    pthread_once(&s_UDTUnitedOnce, reinterpret_cast<void (*)()>(getInstance));
-    return *getInstance();
-}
-
-#endif
-
 SRT_TSA_DISABLED
 void CUDT::construct()
 {

srtcore/packetfilter.cpp

@@ -279,33 +279,12 @@ PacketFilter::Factory::~Factory()
 {
 }
 
-#if HAVE_CXX11
-
 PacketFilter::Internal& PacketFilter::internal()
 {
     static PacketFilter::Internal instance;
     return instance;
 }
 
-#else // !HAVE_CXX11
-
-static pthread_once_t s_PacketFactoryOnce = PTHREAD_ONCE_INIT;
-
-static PacketFilter::Internal *getInstance()
-{
-    static PacketFilter::Internal instance;
-    return &instance;
-}
-
-PacketFilter::Internal& PacketFilter::internal()
-{
-    // We don't want lock each time, pthread_once can be faster than mutex.
-    pthread_once(&s_PacketFactoryOnce, reinterpret_cast<void (*)()>(getInstance));
-    return *getInstance();
-}
-
-#endif
-
 PacketFilter::Internal::Internal()
 {
     // Add here builtin packet filters and mark them

srtcore/sync.cpp

@@ -26,6 +26,18 @@
 #include <random>
 #endif
 
+// IMPORTANT NOTE about the local static variables initialized in the multithreaded
+// environment:
+// 1. C++11 has added a guarantee that a local static variable is always safely
+// initialized, with a deadlock-free guarnatee.
+// 2. C++03 STANDARD doesn't give any such guarantees, however compilers do:
+// - gcc, clang, Intel: They do implement the guarantees required by C++11 and
+//   apply these guarantees even in C++03, unless overridden by -fno-threadsafe-statics
+// - Microsoft Visual Studio: this is supported since VS 2019
+//
+// Therefore this code relies everywhere on that the static locals are initialized
+// safely in the multithreaded environment and pthread_once() is not in use.
+
 using namespace srt::logging;
 using namespace std;
 
@@ -331,74 +343,67 @@ bool CGlobEvent::waitForEvent()
 ////////////////////////////////////////////////////////////////////////////////
 
 #if HAVE_CXX11
-static std::mt19937& randomGen()
+int getRandomInt(int minval, int maxval)
 {
-    static std::random_device s_RandomDevice;
-    static std::mt19937 s_GenMT19937(s_RandomDevice());
-    return s_GenMT19937;
+    thread_local std::random_device s_RandomDevice;
+    thread_local std::mt19937 s_GenMT19937(s_RandomDevice());
+    uniform_int_distribution<> dis(minVal, maxVal);
+    return dis(s_GenMT19937);
 }
-#elif defined(_WIN32) && defined(__MINGW32__)
-static void initRandSeed()
+
+#else
+
+#if SRT_HAVE_RAND_R
+static int randWithSeed()
 {
-    const int64_t seed = sync::steady_clock::now().time_since_epoch().count();
-    srand((unsigned int) seed);
+    static unsigned int s_uRandSeed = sync::steady_clock::now().time_since_epoch().count();
+    return rand_r(&s_uRandSeed);
 }
-static pthread_once_t s_InitRandSeedOnce = PTHREAD_ONCE_INIT;
 #else
+// Mainly MinGW has no rand_r().
+// IMPORTNAT: This poses a risk for applications that use SRT
+// and they use rand() themselves, or some included library does.
 
-static unsigned int genRandSeed()
+// We need a wrapper for srand() because it returns void so we need
+// to fake that it has produced some return value to initialize a static variable.
+static inline unsigned int srandWrapper(unsigned int seed)
 {
-    // Duration::count() does not depend on any global objects,
-    // therefore it is preferred over count_microseconds(..).
-    const int64_t seed = sync::steady_clock::now().time_since_epoch().count();
-    return (unsigned int) seed;
+    srand(seed);
+    return seed;
 }
-
-static unsigned int* getRandSeed()
+static int randWithSeed()
 {
-    static unsigned int s_uRandSeed = genRandSeed();
-    return &s_uRandSeed;
+    static unsigned int s_copyseed = srandWrapper(sync::steady_clock::now().time_since_epoch().count());
+    (void)s_copyseed; // fake it is used
+    return rand();
 }
-
-#endif
+#endif // Mingw || others
 
 int genRandomInt(int minVal, int maxVal)
 {
-    // This Meyers singleton initialization is thread-safe since C++11, but is not thread-safe in C++03.
+    // This Meyers singleton initialization is thread-safe since C++11, but is not thread-safe in C++03
+    // (still, see static initialization note in the beginning).
     // A mutex to protect simultaneous access to the random device.
     // Thread-local storage could be used here instead to store the seed / random device.
     // However the generator is not used often (Initial Socket ID, Initial sequence number, FileCC),
     // so sharing a single seed among threads should not impact the performance.
     static sync::Mutex s_mtxRandomDevice;
     sync::ScopedLock lck(s_mtxRandomDevice);
-#if HAVE_CXX11
-    uniform_int_distribution<> dis(minVal, maxVal); 
-    return dis(randomGen());
-#else
-#if defined(__MINGW32__)
-    // No rand_r(..) for MinGW.
-    pthread_once(&s_InitRandSeedOnce, initRandSeed);
-    // rand() returns a pseudo-random integer in the range 0 to RAND_MAX inclusive
-    // (i.e., the mathematical range [0, RAND_MAX]). 
-    // Therefore, rand_0_1 belongs to [0.0, 1.0].
-    const double rand_0_1 = double(rand()) / RAND_MAX;
-#else // not __MINGW32__
-    // rand_r(..) returns a pseudo-random integer in the range 0 to RAND_MAX inclusive
+
+    // randWithSeed() returns a pseudo-random integer in the range 0 to RAND_MAX inclusive
     // (i.e., the mathematical range [0, RAND_MAX]). 
     // Therefore, rand_0_1 belongs to [0.0, 1.0].
-    const double rand_0_1 = double(rand_r(getRandSeed())) / RAND_MAX;
-#endif
-
+    double rand_0_1 = double(randWithSeed()) / RAND_MAX;
     // Map onto [minVal, maxVal].
     // Note. There is a minuscule probablity to get maxVal+1 as the result.
     // So we have to use long long to handle cases when maxVal = INT32_MAX.
     // Also we must check 'res' does not exceed maxVal,
     // which may happen if rand_0_1 = 1, even though the chances are low.
     const long long llMaxVal = maxVal;
     const int res = minVal + static_cast<int>((llMaxVal + 1 - minVal) * rand_0_1);
-    return min(res, maxVal);
-#endif // HAVE_CXX11
+    return std::min(res, maxVal);
 }
+#endif
 
 #if defined(SRT_ENABLE_STDCXX_SYNC) && HAVE_CXX17