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/7] 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/7] 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/7] 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): From 79653ffb4d7964801e4fc83a0a3497d150976268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edouard=20Choini=C3=A8re?= <27212526+echoix@users.noreply.github.com> Date: Fri, 17 Jul 2026 09:28:02 +0000 Subject: [PATCH 4/7] style: Format cmake files with Gersemi --- lib/gis/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gis/CMakeLists.txt b/lib/gis/CMakeLists.txt index 3899e26d972..9193736a75d 100644 --- a/lib/gis/CMakeLists.txt +++ b/lib/gis/CMakeLists.txt @@ -8,7 +8,7 @@ find_package(Threads) set(grass_gis_DEFS "-DGRASS_VERSION_DATE=\"${GRASS_VERSION_DATE}\"") if(Threads_FOUND) - list(APPEND grass_gis_DEFS "HAVE_PTHREAD") + list(APPEND grass_gis_DEFS "HAVE_PTHREAD") endif() if(MSVC) From 38ed9dc82d9f6c67ec53f45c6fc4a8fd4d75e0b4 Mon Sep 17 00:00:00 2001 From: Vaclav Petras Date: Tue, 18 Aug 2026 11:50:22 -0400 Subject: [PATCH 5/7] libgis: Make random number generation thread safe with C11 atomics Replace the pthread mutex approach with a lock-free compare-and-swap on the whole 48-bit generator state kept in a single C11 atomic integer. One successful compare-and-swap is exactly one step of the standard drand48 linear congruential generator, so the sequence of generated values for a given seed is identical to a single-threaded run and to the previous limb-based implementation. Compared to the mutex: the feature is gated by the compiler (__STDC_NO_ATOMICS__), not by --with-pthread, so it is active in every supported build, including the OpenMP-without-pthread configurations (macOS CI, Docker images) where the race reported in #5742 actually occurs; no build system changes or extra link dependencies are needed (the Makefile and CMake changes are dropped, which also avoids defining HAVE_PTHREAD from the Win32-threads CMake package on MSVC); and there is no lock contention (in a microbenchmark the mutex made threaded runs slower than single-threaded at every thread count, while the atomic version is about twice as fast as the mutex when several threads generate numbers at the same time). A single shared generator still cannot scale with the number of threads; per-thread state remains future work. Where C11 atomics are unavailable, the code falls back to the previous plain implementation, which is not thread-safe (unchanged behavior). Output compatibility is guarded by the PRNG regression tests merged in #7633 and #7634 (lib/gis/tests/lib_gis_lrand48_test.py and raster/r.mapcalc/tests/r_mapcalc_rand_seed_test.py) and was additionally verified against the previous implementation for multiple seeds. The 64-bit reformulation of the generator and its verification were AI-assisted (Claude). --- lib/gis/CMakeLists.txt | 6 -- lib/gis/Makefile | 2 +- lib/gis/lrand48.c | 151 +++++++++++++++++++----------- lib/gis/testsuite/test_lrand48.py | 53 +++-------- 4 files changed, 111 insertions(+), 101 deletions(-) diff --git a/lib/gis/CMakeLists.txt b/lib/gis/CMakeLists.txt index 27cd726c94f..fe9d7d6750d 100644 --- a/lib/gis/CMakeLists.txt +++ b/lib/gis/CMakeLists.txt @@ -4,13 +4,7 @@ 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 "${PROJECT_SOURCE_DIR}/msvc") diff --git a/lib/gis/Makefile b/lib/gis/Makefile index f2d2ba0292a..90b39f374a4 100644 --- a/lib/gis/Makefile +++ b/lib/gis/Makefile @@ -2,7 +2,7 @@ MODULE_TOPDIR = ../.. LIB = GIS -LIBES = $(OPENMP_LIBPATH) $(OPENMP_LIB) $(PTHREADLIB) +LIBES = $(OPENMP_LIBPATH) $(OPENMP_LIB) 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 b7ac8ac136f..6c2236c2b3e 100644 --- a/lib/gis/lrand48.c +++ b/lib/gis/lrand48.c @@ -3,6 +3,21 @@ * * \brief GIS Library - Pseudo-random number generation * + * The generator is the standard drand48 linear congruential generator + * X' = (A * X + B) mod 2^48 with A = 0x5DEECE66D and B = 0xB. + * + * When C11 atomic operations are available, the generator state is + * advanced with an atomic compare-and-swap and the generating functions + * are thread-safe: the sequence of generated values for a given seed is + * the same as in a single-threaded run. Which thread receives which + * value depends on scheduling, so results are fully reproducible only + * with single-threaded execution. Without C11 atomics (notably MSVC, + * which defines __STDC_NO_ATOMICS__), the generator falls back to plain + * state updates, so multi-threaded usage is safe only when compiled + * with C11 atomics. + * + * The seeding functions are not thread-safe; see G_srand48(). + * * (C) 2014-2025 by the GRASS Development Team * * This program is free software under the GNU General Public License @@ -16,13 +31,18 @@ #include #include +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L && \ + !defined(__STDC_NO_ATOMICS__) +#include +#include +#define LRAND48_ATOMIC 1 +#else +#define LRAND48_ATOMIC 0 +#endif + #include #include -#ifdef HAVE_PTHREAD -#include -#endif - #ifdef HAVE_GETTIMEOFDAY #include #else @@ -36,6 +56,21 @@ typedef unsigned short uint16; typedef unsigned int uint32; typedef signed int int32; +#if LRAND48_ATOMIC + +/* The whole 48-bit state is kept in one atomic integer so that it can be + * advanced in one compare-and-swap: a successful swap is exactly one + * generator step, giving the same sequence of states as a single-threaded + * run. The multiplication may wrap around at 2^64; that does not change + * the result modulo 2^48. */ +static atomic_uint_least64_t state; + +#define LCG_A UINT64_C(0x5DEECE66D) +#define LCG_B UINT64_C(0xB) +#define MASK48 UINT64_C(0xFFFFFFFFFFFF) + +#else + static uint16 x0, x1, x2; static const uint32 a0 = 0xE66D; static const uint32 a1 = 0xDEEC; @@ -43,11 +78,9 @@ static const uint32 a2 = 0x5; static const uint32 b0 = 0xB; -static int seeded; +#endif /* LRAND48_ATOMIC */ -#ifdef HAVE_PTHREAD -static pthread_mutex_t lrand48_mutex = PTHREAD_MUTEX_INITIALIZER; -#endif +static int seeded; #define LO(x) ((x) & 0xFFFFU) #define HI(x) ((x) >> 16) @@ -55,28 +88,24 @@ static pthread_mutex_t lrand48_mutex = PTHREAD_MUTEX_INITIALIZER; /*! * \brief Seed the pseudo-random number generator * - * \param[in] seedval 32-bit integer used to seed the PRNG + * This function is not thread-safe. In a multi-threaded program, call + * `G_srand48()` once *before* starting the worker threads; it must not + * run concurrently with another thread seeding or generating values. * - * 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. + * \param[in] seedval 32-bit integer used to seed the PRNG */ void G_srand48(long seedval) { uint32 x = (uint32) * (unsigned long *)&seedval; -#ifdef HAVE_PTHREAD - pthread_mutex_lock(&lrand48_mutex); -#endif - +#if LRAND48_ATOMIC + atomic_store(&state, ((uint_least64_t)x << 16) | 0x330E); +#else x2 = (uint16)HI(x); x1 = (uint16)LO(x); x0 = (uint16)0x330E; - seeded = 1; - -#ifdef HAVE_PTHREAD - pthread_mutex_unlock(&lrand48_mutex); #endif + seeded = 1; } /*! @@ -85,9 +114,9 @@ void G_srand48(long seedval) * A weak hash of the current time and PID is generated and used to * seed the PRNG * - * 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. + * This function is not thread-safe. In a multi-threaded program, call + * `G_srand48_auto()` once *before* starting the worker threads; it must + * not run concurrently with another thread seeding or generating values. * * \return generated seed value passed to G_srand48() */ @@ -126,6 +155,29 @@ long G_srand48_auto(void) return (long)seed; } +#if LRAND48_ATOMIC + +/* Advance the generator by one step and return the new state. Callers + * derive their result from the returned value, not from shared state, so + * concurrent calls each get a distinct step of the sequence. */ +static uint_least64_t G__next(void) +{ + uint_least64_t cur = atomic_load_explicit(&state, memory_order_relaxed); + uint_least64_t next; + + if (!seeded) + G_fatal_error(_("Pseudo-random number generator not seeded")); + + do { + next = (LCG_A * cur + LCG_B) & MASK48; + } while (!atomic_compare_exchange_weak_explicit( + &state, &cur, next, memory_order_relaxed, memory_order_relaxed)); + + return next; +} + +#else + static void G__next(void) { uint32 a0x0 = a0 * x0; @@ -149,71 +201,68 @@ static void G__next(void) x2 = (uint16)LO(y2); } +#endif /* LRAND48_ATOMIC */ + /*! * \brief Generate an integer in the range [0, 2^31) * - * This function is thread-safe. + * This function is thread-safe only when compiled with C11 atomics + * (see the comment at the top of the file). * * \return the generated value */ long G_lrand48(void) { +#if LRAND48_ATOMIC + return (long)(G__next() >> 17); +#else 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; +#endif } /*! * \brief Generate an integer in the range [-2^31, 2^31) * - * This function is thread-safe. + * This function is thread-safe only when compiled with C11 atomics + * (see the comment at the top of the file). * * \return the generated value */ long G_mrand48(void) { - uint32 r; +#if LRAND48_ATOMIC + uint32 r = (uint32)(G__next() >> 16); -#ifdef HAVE_PTHREAD - pthread_mutex_lock(&lrand48_mutex); -#endif + return (long)(int32)r; +#else + uint32 r; G__next(); r = ((uint32)x2 << 16) | ((uint32)x1); - -#ifdef HAVE_PTHREAD - pthread_mutex_unlock(&lrand48_mutex); -#endif - return (long)(int32)r; +#endif } /*! * \brief Generate a floating-point value in the range [0,1) * - * This function is thread-safe. + * This function is thread-safe only when compiled with C11 atomics + * (see the comment at the top of the file). * * \return the generated value */ double G_drand48(void) { +#if LRAND48_ATOMIC + /* The state is below 2^53, so the conversion to double is exact. */ + return (double)G__next() / 281474976710656.0; /* 2^48 */ +#else double r = 0.0; -#ifdef HAVE_PTHREAD - pthread_mutex_lock(&lrand48_mutex); -#endif - G__next(); r += x2; r *= 0x10000; @@ -221,12 +270,8 @@ double G_drand48(void) r *= 0x10000; r += x0; r /= 281474976710656.0; /* 2^48 */ - -#ifdef HAVE_PTHREAD - pthread_mutex_unlock(&lrand48_mutex); -#endif - return r; +#endif } /* diff --git a/lib/gis/testsuite/test_lrand48.py b/lib/gis/testsuite/test_lrand48.py index 157baab1d24..e0cfdccc59a 100644 --- a/lib/gis/testsuite/test_lrand48.py +++ b/lib/gis/testsuite/test_lrand48.py @@ -1,7 +1,6 @@ """Test of gis library lrand48 PRNG thread-safety @author Maris Nartiss -@author Gemini @copyright 2025 by the GRASS Development Team @@ -10,7 +9,6 @@ for details """ -import ctypes import threading from grass.gunittest.case import TestCase @@ -30,26 +28,11 @@ def test_thread_safety_and_reproducibility(self): 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() @@ -57,14 +40,15 @@ 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 + # The lock protects only the Python result list, not the C + # generator under test (list.extend is atomic under the GIL, + # but that is a CPython implementation detail). with lock: list_multi_raw.extend(local_results) - # Reset the seed to ensure the sequence starts from the beginning + # Reset the seed to ensure the sequence starts from the beginning. G_srand48(seed) threads = [] @@ -76,30 +60,17 @@ def worker(): 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. + # The generator serializes state updates, so the threads together + # must consume exactly the single-threaded sequence; only the + # distribution of values between threads may differ. Sorting both + # lists removes the scheduling-dependent order before comparison. + # A set-based comparison would not work, because a correct + # sequence can contain legitimate duplicates. self.assertListEqual( sorted(list_single), sorted(list_multi_raw), - "The set of generated numbers differs between single-threaded and multi-threaded runs.", + "The set of generated numbers differs between " + "single-threaded and multi-threaded runs.", ) From f8525066d738beeef8884c667dd2f602d719b822 Mon Sep 17 00:00:00 2001 From: Nicklas Larsson Date: Thu, 10 Sep 2026 16:20:58 +0200 Subject: [PATCH 6/7] Update lib/gis/testsuite/test_lrand48.py --- lib/gis/testsuite/test_lrand48.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/gis/testsuite/test_lrand48.py b/lib/gis/testsuite/test_lrand48.py index e0cfdccc59a..b642019d8ae 100644 --- a/lib/gis/testsuite/test_lrand48.py +++ b/lib/gis/testsuite/test_lrand48.py @@ -2,12 +2,9 @@ @author Maris Nartiss -@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 -""" + SPDX-FileCopyrightText: 2025 GRASS Development Team + SPDX-License-Identifier: GPL-2.0-or-later + """ import threading From 7bd84bde1562bc425c35ef9881659336f32ad413 Mon Sep 17 00:00:00 2001 From: Nicklas Larsson Date: Thu, 10 Sep 2026 16:22:01 +0200 Subject: [PATCH 7/7] Apply suggestion --- lib/gis/testsuite/test_lrand48.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gis/testsuite/test_lrand48.py b/lib/gis/testsuite/test_lrand48.py index b642019d8ae..f63f30eef9d 100644 --- a/lib/gis/testsuite/test_lrand48.py +++ b/lib/gis/testsuite/test_lrand48.py @@ -2,9 +2,9 @@ @author Maris Nartiss - SPDX-FileCopyrightText: 2025 GRASS Development Team - SPDX-License-Identifier: GPL-2.0-or-later - """ +SPDX-FileCopyrightText: 2025 GRASS Development Team +SPDX-License-Identifier: GPL-2.0-or-later +""" import threading