-
Notifications
You must be signed in to change notification settings - Fork 730
Add thread-local random number generator for thread safety #423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduces PS_THREAD_LOCAL macro that provides platform-agnostic thread-local storage support using compiler-specific keywords: - _Thread_local for C11 - thread_local for C++11 - __thread for GCC/Clang - __declspec(thread) for MSVC This abstraction allows for consistent thread-local storage usage across different compilers and platforms.
Updates the Mersenne Twister RNG implementation to use thread-local storage for its state variables (mt[] array and mti index). This eliminates race conditions when multiple threads use the RNG concurrently. Changes: - Make mt[N] array thread-local using PS_THREAD_LOCAL - Make mti index thread-local using PS_THREAD_LOCAL - Add thread safety documentation to header - Maintain backward compatibility - API unchanged When PS_USE_THREAD_LOCAL_RNG is defined (set via CMake), each thread gets its own RNG state, preventing data races and ensuring deterministic behavior in multi-threaded applications.
Introduces portable thread utilities to enable multi-threaded testing across different platforms: - Thread creation and joining - Mutexes for synchronization - Barriers for thread coordination - Platform detection for Windows (threads.h) vs POSIX (pthread) - macOS-specific pthread barrier emulation These utilities enable comprehensive testing of thread safety in the RNG implementation and other concurrent code paths.
Adds test suite to verify thread safety of the RNG implementation: Thread-local storage tests: - test_thread_local_compile.c: Verifies TLS compilation support - test_thread_local_basic.c: Tests thread isolation of TLS variables RNG thread safety tests: - test_genrand_baseline.c: Baseline single-threaded RNG behavior - test_genrand_thread.c: Demonstrates race conditions without TLS - test_genrand_thread_tls.c: Verifies thread safety with TLS enabled The tests use collision detection to identify when threads share state inappropriately, and verify that the TLS implementation eliminates these race conditions.
Updates build system to support thread-local RNG compilation: - Add PS_THREAD_LOCAL_RNG option (enabled by default) - Detect compiler support for thread-local storage - Configure PS_USE_THREAD_LOCAL_RNG macro when enabled - Add thread utilities library for tests - Register new test executables with CTest - Link tests with thread utilities and pthread where needed The feature can be disabled with -DPS_THREAD_LOCAL_RNG=OFF if thread-local storage is not available or desired.
Documents the thread-local RNG implementation and provides guidance for multi-threaded usage of PocketSphinx: - Explains the thread safety issue with global RNG state - Describes the thread-local storage solution - Provides migration guide for existing code - Lists performance considerations - Documents the PS_THREAD_LOCAL_RNG build option - Includes examples of thread-safe usage This helps users understand and properly utilize the thread-safe RNG functionality in their applications.
dhdaines
approved these changes
Jul 26, 2025
Contributor
dhdaines
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Of course, Arthur's comment implies that we are already 11 years overdue for replacing the RNG with another algorithm!
Do you know if this works on musl libc based systems? I think we can use docker to test this in the CI...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make random number generator thread-safe using thread-local storage
Summary
This PR makes the Mersenne Twister random number generator (RNG) in PocketSphinx thread-safe by using thread-local storage (TLS) for its state variables. This eliminates race conditions and data corruption when multiple threads use the RNG concurrently.
Problem
The RNG implementation used global static variables (
mt[]array andmtiindex) that were shared across all threads, causing:Solution
PS_THREAD_LOCALmacro)Key Features
✅ Fully backward compatible - No code changes required for users
✅ Thread-safe by default - Each thread gets independent RNG state
✅ Cross-platform support - Works with C11, C++11, GCC, Clang, MSVC
✅ Opt-out available - Can be disabled with
-DPS_THREAD_LOCAL_RNG=OFF✅ Comprehensive testing - Includes thread safety verification tests
Implementation Details
The implementation adds
PS_THREAD_LOCALbefore the static RNG state:When
PS_USE_THREAD_LOCAL_RNGis defined (default), the macro expands to the appropriate thread-local keyword for the compiler. When disabled, it expands to nothing, reverting to the original behavior.Testing
New tests verify:
Run thread safety tests:
Build Configuration
The feature is enabled by default but can be disabled:
Commits
feat: Add thread-local storage abstraction header- Platform-agnostic TLS supportfeat: Make random number generator thread-safe using TLS- Core RNG changestest: Add cross-platform thread utilities for testing- Testing infrastructuretest: Add comprehensive tests for thread-local RNG- Thread safety verificationbuild: Add CMake support for thread-local RNG feature- Build system integrationdocs: Add thread safety documentation- User documentationImpact
This is a bug fix that makes the code thread-safe without breaking any properly-functioning existing code.
Files Changed
src/util/thread_local.h- New TLS abstraction headersrc/util/genrand.c- Added TLS to RNG state variablessrc/util/genrand.h- Added thread safety documentationCMakeLists.txt- Added PS_THREAD_LOCAL_RNG optionconfig.h.in- Added PS_USE_THREAD_LOCAL_RNG configurationtest/unit/- Added comprehensive thread safety testsdocs/thread_safety.md- New documentation for thread safety