From 5ccafac9e5e87630213c1515aff902ef96c380d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C4=81ris=20Narti=C5=A1s?= Date: Fri, 10 Oct 2025 12:00:59 +0300 Subject: [PATCH 1/3] lib: correctly guard random number generation when run threaded As random number generator tracks its internal state as a static variable, it is necessary to guard its updates with mutex to prevent misbehavour when multiple threads change values simultaneously --- lib/gis/CMakeLists.txt | 6 ++++++ lib/gis/Makefile | 2 +- lib/gis/lrand48.c | 38 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/lib/gis/CMakeLists.txt b/lib/gis/CMakeLists.txt index 711725b02b3..2739ca78511 100644 --- a/lib/gis/CMakeLists.txt +++ b/lib/gis/CMakeLists.txt @@ -4,7 +4,13 @@ if(NOT WIN32) list(FILTER gislib_SRCS EXCLUDE REGEX [[.*/fmode\.c$]]) endif() +find_package(Threads) + set(grass_gis_DEFS "-DGRASS_VERSION_DATE=\"${GRASS_VERSION_DATE}\"") +if(Threads_FOUND) + list(APPEND grass_gis_DEFS "HAVE_PTHREAD") +endif() + if(MSVC) set(grass_gis_DEFS "${grass_gis_DEFS};-D_USE_MATH_DEFINES=1") set(gislib_INCLUDES "../../msvc") diff --git a/lib/gis/Makefile b/lib/gis/Makefile index 90b39f374a4..f2d2ba0292a 100644 --- a/lib/gis/Makefile +++ b/lib/gis/Makefile @@ -2,7 +2,7 @@ MODULE_TOPDIR = ../.. LIB = GIS -LIBES = $(OPENMP_LIBPATH) $(OPENMP_LIB) +LIBES = $(OPENMP_LIBPATH) $(OPENMP_LIB) $(PTHREADLIB) EXTRA_INC = $(ZLIBINCPATH) $(BZIP2INCPATH) $(ZSTDINCPATH) $(PTHREADINCPATH) $(REGEXINCPATH) $(OPENMP_INCPATH) EXTRA_CFLAGS = $(OPENMP_CFLAGS) -DGRASS_VERSION_DATE=\"'$(GRASS_VERSION_DATE)'\" diff --git a/lib/gis/lrand48.c b/lib/gis/lrand48.c index 63d77d9fb99..ee52a67fc80 100644 --- a/lib/gis/lrand48.c +++ b/lib/gis/lrand48.c @@ -3,12 +3,12 @@ * * \brief GIS Library - Pseudo-random number generation * - * (C) 2014 by the GRASS Development Team + * (C) 2014, 2025 by the GRASS Development Team * * This program is free software under the GNU General Public License * (>=v2). Read the file COPYING that comes with GRASS for details. * - * \author Glynn Clements + * \authors Glynn Clements, Maris Nartiss (thread safety) */ #include @@ -19,6 +19,10 @@ #include #include +#ifdef HAVE_PTHREAD +#include +#endif + #ifdef HAVE_GETTIMEOFDAY #include #else @@ -41,6 +45,10 @@ static const uint32 b0 = 0xB; static int seeded; +#ifdef HAVE_PTHREAD +static pthread_mutex_t lrand48_mutex = PTHREAD_MUTEX_INITIALIZER; +#endif + #define LO(x) ((x) & 0xFFFFU) #define HI(x) ((x) >> 16) @@ -48,15 +56,25 @@ static int seeded; * \brief Seed the pseudo-random number generator * * \param[in] seedval 32-bit integer used to seed the PRNG + * + * This function is thread-safe. */ void G_srand48(long seedval) { uint32 x = (uint32) * (unsigned long *)&seedval; +#ifdef HAVE_PTHREAD + pthread_mutex_lock(&lrand48_mutex); +#endif + x2 = (uint16)HI(x); x1 = (uint16)LO(x); x0 = (uint16)0x330E; seeded = 1; + +#ifdef HAVE_PTHREAD + pthread_mutex_unlock(&lrand48_mutex); +#endif } /*! @@ -65,6 +83,8 @@ void G_srand48(long seedval) * A weak hash of the current time and PID is generated and used to * seed the PRNG * + * This function is thread-safe. + * * \return generated seed value passed to G_srand48() */ long G_srand48_auto(void) @@ -104,6 +124,10 @@ long G_srand48_auto(void) static void G__next(void) { +#ifdef HAVE_PTHREAD + pthread_mutex_lock(&lrand48_mutex); +#endif + uint32 a0x0 = a0 * x0; uint32 a0x1 = a0 * x1; uint32 a0x2 = a0 * x2; @@ -123,11 +147,17 @@ static void G__next(void) x1 = (uint16)LO(y1); y2 += HI(y1); x2 = (uint16)LO(y2); + +#ifdef HAVE_PTHREAD + pthread_mutex_unlock(&lrand48_mutex); +#endif } /*! * \brief Generate an integer in the range [0, 2^31) * + * This function is thread-safe. + * * \return the generated value */ long G_lrand48(void) @@ -142,6 +172,8 @@ long G_lrand48(void) /*! * \brief Generate an integer in the range [-2^31, 2^31) * + * This function is thread-safe. + * * \return the generated value */ long G_mrand48(void) @@ -156,6 +188,8 @@ long G_mrand48(void) /*! * \brief Generate a floating-point value in the range [0,1) * + * This function is thread-safe. + * * \return the generated value */ double G_drand48(void) From 2c4592d22fef34f83e89dec7290d7f62de35eeec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C4=81ris=20Narti=C5=A1s?= Date: Mon, 20 Oct 2025 10:19:11 +0300 Subject: [PATCH 2/3] lib: ensure reproducible stream of random numbers when called from multiple threads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As random number generation is guarded by a mutex, it makes generation a serial operation – always leading to the same sequence of numbers --- lib/gis/lrand48.c | 45 ++++++++++--- lib/gis/testsuite/test_lrand48.py | 108 ++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 11 deletions(-) create mode 100644 lib/gis/testsuite/test_lrand48.py diff --git a/lib/gis/lrand48.c b/lib/gis/lrand48.c index ee52a67fc80..b7ac8ac136f 100644 --- a/lib/gis/lrand48.c +++ b/lib/gis/lrand48.c @@ -3,7 +3,7 @@ * * \brief GIS Library - Pseudo-random number generation * - * (C) 2014, 2025 by the GRASS Development Team + * (C) 2014-2025 by the GRASS Development Team * * This program is free software under the GNU General Public License * (>=v2). Read the file COPYING that comes with GRASS for details. @@ -57,7 +57,9 @@ static pthread_mutex_t lrand48_mutex = PTHREAD_MUTEX_INITIALIZER; * * \param[in] seedval 32-bit integer used to seed the PRNG * - * This function is thread-safe. + * In a multi-threaded program, call `G_srand48()` once from the main + * thread *before* starting the worker threads. Subsequent calls will + * reset global seed state used by all threads. */ void G_srand48(long seedval) { @@ -83,7 +85,9 @@ void G_srand48(long seedval) * A weak hash of the current time and PID is generated and used to * seed the PRNG * - * This function is thread-safe. + * In a multi-threaded program, call `G_srand48_auto()` once from the main + * thread *before* starting the worker threads. Subsequent calls will + * reset global seed state used by all threads. * * \return generated seed value passed to G_srand48() */ @@ -124,10 +128,6 @@ long G_srand48_auto(void) static void G__next(void) { -#ifdef HAVE_PTHREAD - pthread_mutex_lock(&lrand48_mutex); -#endif - uint32 a0x0 = a0 * x0; uint32 a0x1 = a0 * x1; uint32 a0x2 = a0 * x2; @@ -147,10 +147,6 @@ static void G__next(void) x1 = (uint16)LO(y1); y2 += HI(y1); x2 = (uint16)LO(y2); - -#ifdef HAVE_PTHREAD - pthread_mutex_unlock(&lrand48_mutex); -#endif } /*! @@ -164,8 +160,17 @@ long G_lrand48(void) { uint32 r; +#ifdef HAVE_PTHREAD + pthread_mutex_lock(&lrand48_mutex); +#endif + G__next(); r = ((uint32)x2 << 15) | ((uint32)x1 >> 1); + +#ifdef HAVE_PTHREAD + pthread_mutex_unlock(&lrand48_mutex); +#endif + return (long)r; } @@ -180,8 +185,17 @@ long G_mrand48(void) { uint32 r; +#ifdef HAVE_PTHREAD + pthread_mutex_lock(&lrand48_mutex); +#endif + G__next(); r = ((uint32)x2 << 16) | ((uint32)x1); + +#ifdef HAVE_PTHREAD + pthread_mutex_unlock(&lrand48_mutex); +#endif + return (long)(int32)r; } @@ -196,6 +210,10 @@ double G_drand48(void) { double r = 0.0; +#ifdef HAVE_PTHREAD + pthread_mutex_lock(&lrand48_mutex); +#endif + G__next(); r += x2; r *= 0x10000; @@ -203,6 +221,11 @@ double G_drand48(void) r *= 0x10000; r += x0; r /= 281474976710656.0; /* 2^48 */ + +#ifdef HAVE_PTHREAD + pthread_mutex_unlock(&lrand48_mutex); +#endif + return r; } diff --git a/lib/gis/testsuite/test_lrand48.py b/lib/gis/testsuite/test_lrand48.py new file mode 100644 index 00000000000..f3b1c9eb90c --- /dev/null +++ b/lib/gis/testsuite/test_lrand48.py @@ -0,0 +1,108 @@ +"""Test of gis library lrand48 PRNG thread-safety + +@author Maris Nartiss +@author Gemini + +@copyright 2025 by the GRASS Development Team + +@license This program is free software under the GNU General Public License (>=v2). +Read the file COPYING that comes with GRASS +for details +""" + +import ctypes +import threading + +from grass.gunittest.case import TestCase +from grass.gunittest.main import test + +from grass.lib.gis import G_srand48, G_lrand48 + + +class Lrand48ThreadSafetyTestCase(TestCase): + """Test case for lrand48 thread-safety and reproducibility.""" + + def test_thread_safety_and_reproducibility(self): + """Verify that multi-threaded execution produces the same set of + random numbers as single-threaded execution.""" + + seed = 1337 + num_values = 10000 + num_threads = 4 + values_per_thread = num_values // num_threads + + self.assertEqual( + num_values % num_threads, + 0, + "Total number of values must be divisible by the number of threads.", + ) + + # --- Define ctypes function signatures --- + G_srand48.argtypes = [ctypes.c_long] + G_srand48.restype = None + + G_lrand48.argtypes = [] + G_lrand48.restype = ctypes.c_long + + # --- 1. Single-threaded execution --- + list_single = [] + G_srand48(seed) + for _ in range(num_values): + list_single.append(G_lrand48()) + + # --- 2. Multi-threaded execution --- + list_multi_raw = [] + lock = threading.Lock() + + def worker(): + """Calls G_lrand48 and appends the result to a shared list.""" + local_results = [] + for _ in range(values_per_thread): + # G_lrand48() itself is protected by a C-level mutex + local_results.append(G_lrand48()) + + # Use a Python-level lock to safely extend the shared list + with lock: + list_multi_raw.extend(local_results) + + # Reset the seed to ensure the sequence starts from the beginning + G_srand48(seed) + + threads = [] + for _ in range(num_threads): + thread = threading.Thread(target=worker) + threads.append(thread) + thread.start() + + for thread in threads: + thread.join() + + # --- 3. Verification --- + self.assertEqual( + len(list_single), + len(list_multi_raw), + "Single-threaded and multi-threaded runs produced a different number of values.", + ) + + # Check for duplicates in the multi-threaded list. The presence of duplicates + # would indicate that the C-level mutex failed and multiple threads + # received the same random number. + self.assertEqual( + len(list_multi_raw), + len(set(list_multi_raw)), + "Duplicate values found in multi-threaded run, indicating a race condition.", + ) + + # The sorted lists of numbers must be identical. + # This confirms that although threads ran in parallel, the C-level mutex + # correctly serialized access to the PRNG, yielding the exact same + # block of numbers. + self.assertListEqual( + sorted(list_single), + sorted(list_multi_raw), + "The set of generated numbers differs between single-threaded and multi-threaded runs.", + ) + + +if __name__ == "__main__": + test() From f057b6798c7d4378be5960824057733139e2826b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edouard=20Choini=C3=A8re?= <27212526+echoix@users.noreply.github.com> Date: Sun, 30 Nov 2025 15:59:23 -0500 Subject: [PATCH 3/3] Update lib/gis/testsuite/test_lrand48.py Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- lib/gis/testsuite/test_lrand48.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/gis/testsuite/test_lrand48.py b/lib/gis/testsuite/test_lrand48.py index f3b1c9eb90c..157baab1d24 100644 --- a/lib/gis/testsuite/test_lrand48.py +++ b/lib/gis/testsuite/test_lrand48.py @@ -15,8 +15,7 @@ from grass.gunittest.case import TestCase from grass.gunittest.main import test - -from grass.lib.gis import G_srand48, G_lrand48 +from grass.lib.gis import G_lrand48, G_srand48 class Lrand48ThreadSafetyTestCase(TestCase):