Make signal handler registration thread-safe#375
Merged
Conversation
When multiple threads call scs_solve concurrently, each thread calls scs_start_interrupt_listener which writes to global state (int_detected, oact/sigaction) without synchronization, causing data races detected by Thread Sanitizer. Fix by adding reference counting with a mutex (pthread_mutex on Unix, CRITICAL_SECTION on Windows). Only the first concurrent solver installs the signal handler, and only the last one restores it. Use volatile sig_atomic_t for int_detected (POSIX signal safety). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move scs_start_interrupt_listener() after the input validation checks so that early-return paths (NULL data/cone, failed validation) don't increment listener_count without a matching decrement. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
InitOnceExecuteOnce/INIT_ONCE caused segfaults on MSYS2/MinGW-w64. Replace with a simple InterlockedCompareExchange spinlock, which is universally available on all Windows toolchains. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit 0a31905.
The failure() helper calls scs_end_interrupt_listener() unconditionally, including when scs_init fails validation before calling scs_start. Guard against decrementing listener_count below 0, and on Windows call InitOnceExecuteOnce in scs_end too so the CRITICAL_SECTION is always initialized before use. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
When multiple threads call
scs_solveconcurrently, each thread callsscs_start_interrupt_listenerwhich writes to globalint_detectedandstruct sigaction oactwithout synchronization. This causes data races detected by Thread Sanitizer (TSan) when testing free-threaded Python (3.13t/3.14t) with the scs-python wrapper.Fix:
pthread_mutex_t+ reference count. Only the first concurrent solver installs the SIGINT handler, only the last restores it. Changeint_detectedtovolatile sig_atomic_t(POSIX-correct type for signal handler communication).CRITICAL_SECTION(initialized viaInitOnceExecuteOnce) + reference count. UseInterlockedExchange/InterlockedCompareExchangefor atomicint_detectedaccess.find_package(Threads)in CMake and-lpthreadin the Makefile (Linux only — macOS has pthreads built-in).No API changes. The
scs_start_interrupt_listener/scs_end_interrupt_listener/scs_is_interruptedsignatures and behavior are unchanged from the caller's perspective.Context
TSan report from scs-python free-threading CI:
Test plan
🤖 Generated with Claude Code