fix: harden multithreading synchronization and v2 docs - #31
Merged
Conversation
Restore opt-in single-header implementation semantics, synchronize shared thread-pool state under the existing spinlock, make x86 spin waiting portable without AVX, fix the ST/MT force comparator, and update the README for the v2 API.
Owner
|
Looks good. Merging now. |
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
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.
Follow-up to #30 focused on threading correctness, portability, and a few v2 regressions found after the multithreading rewrite.
What changed
Why
The multithreading refactor introduced a few cross-cutting regressions while changing the execution architecture.
In particular, the thread pool was reading cnt, active_jobs, and shutdown_flag concurrently outside the spinlock and relying on volatile. In C, volatile does not provide atomicity or inter-thread synchronization, so those accesses constitute a data race.
The updated implementation keeps the existing spinlock architecture but ensures shared pool state is inspected and modified while holding the lock.
The ST-vs-MT comparison test also printed force mismatches without incrementing its mismatch counter, allowing that check to report success despite differing forces.
Finally, the v2 README example still reflected the previous API and would not correctly initialize the new radix/threading state.
Validation
The final branch passes:
This intentionally leaves the overall threading architecture unchanged and limits the patch to correctness, portability, and documentation fixes.