Skip to content

Fix race in Parallel.start() that could exceed -j worker threads - #4935

Merged
bdbaddog merged 1 commit into
SCons:masterfrom
bdbaddog:fix_flaky_stack_size
Oct 21, 2026
Merged

bdbaddog merged 1 commit into
SCons:masterfrom
bdbaddog:fix_flaky_stack_size

Conversation

@bdbaddog

@bdbaddog bdbaddog commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

The initial worker was started outside tm_lock. The new thread could run _maybe_start_worker() before the main thread appended it to self.workers, see a stale count, and start another worker. This made test/option/stack-size.py intermittently report an extra "Setting stack size failed" warning.

Hold tm_lock around the initial _start_worker() call.

Also remove test/option/stack-size.py branches for Python versions without threading.stack_size, which are no longer supported.

Co-Authored-By: Claude Haiku 5.5 noreply@anthropic.com

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt and RELEASE.txt (and read the README.rst).
  • I have updated the appropriate documentation

The initial worker was started outside tm_lock. The new thread could
run _maybe_start_worker() before the main thread appended it to
self.workers, see a stale count, and start another worker. This made
test/option/stack-size.py intermittently report an extra "Setting
stack size failed" warning.

Hold tm_lock around the initial _start_worker() call.

Also remove test/option/stack-size.py branches for Python versions
without threading.stack_size, which are no longer supported.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
@bdbaddog
bdbaddog requested a review from mwichmann October 19, 2026 03:46
@bdbaddog

Copy link
Copy Markdown
Contributor Author

@acmorrow - claude thinks it found a missing lock in Parallel. thoughts?

@bdbaddog bdbaddog added testsuite Things that only affect the SCons testing. Do not use just because a PR has tests. taskmaster labels Oct 19, 2026
@bdbaddog
bdbaddog merged commit e1b5350 into SCons:master Oct 21, 2026
12 checks passed
@mwichmann mwichmann added this to the NextRelease milestone Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

taskmaster testsuite Things that only affect the SCons testing. Do not use just because a PR has tests.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants