Skip to content

Conversation

@yoney
Copy link
Contributor

@yoney yoney commented Dec 1, 2025

While investigating thread safety in the cmath module, I found that the module is generally thread-safe. However, there is a potential race condition during the initialization of the special value tables in cmath_exec(). This is usually safe, but with subinterpreters, a race condition could occur.

The fix converts all special value tables from runtime initialization to static initialization, eliminating the race condition.

I have also created a test to run under ThreadSanitizer. However, I am not including the test in the current PR because, although this PR addresses the issue in the cmath module, ThreadSanitizer still reports other issues below when subinterpreters and free-threading are used together. I plan to address those issues.

...
SUMMARY: ThreadSanitizer: data race /workspace/cmath_tsan/./Modules/posixmodule.c:18691 in posixmodule_exec
...
SUMMARY: ThreadSanitizer: data race /workspace/cmath_tsan/Objects/exceptions.c:4547 in _PyBuiltins_AddExceptions
...
SUMMARY: ThreadSanitizer: data race /workspace/cmath_tsan/Python/crossinterp_exceptions.h:154 in init_static_exctypes
...
...

cc: @mpage @colesbury

Copy link
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good. It's subinterpreter specific and not really free threading related (i.e., you'll have the same data races with the default GIL enabled build).

@yoney yoney marked this pull request as ready for review December 1, 2025 20:58
@yoney
Copy link
Contributor Author

yoney commented Dec 1, 2025

Thank you for the review, Sam!

It's subinterpreter specific and not really free threading related.

Yes, I agree, this is related to subinterpreters. I was mainly checking the module for ft-python, which is why I created the PR under gh-116738 to mark it checked.

Copy link
Contributor

@skirpichev skirpichev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but could you double check and verify that all functions include tests for all combinations of special values? I.e. when one component is a zero, infinity or nan, same other (or is a finite nonzero number, if the first is nonzero).

Tests are here: Lib/test/mathdata/cmath_testcases.txt (usually in a dedicated section, like "special values").

Test script
#!/bin/sh

# a.sh

file=Lib/test/mathdata/cmath_testcases.txt
funcs="acos acosh asinh atanh cosh exp log sinh sqrt tanh rect"
zeros="0.0 -0.0"
nonfinite="-inf nan inf"
all="$zeros $nonfinite [1-9]\.[0-9].* -[1-9]\.[0-9].*"
for f in $funcs
do
  for t1 in $zeros
  do
    for t2 in $zeros $nonfinite
    do
      re=" $f $t1 $t2 -> "
      grep -q "$re" $file || echo "$f $t1 $t2 - missing"
      re=" $f $t2 $t1 -> "
      grep -q "$re" $file || echo "$f $t2 $t1 - missing"
    done
  done
  for t1 in $nonfinite
  do
    for t2 in $all
    do
      re=" $f $t1 $t2 -> "
      grep -q "$re" $file || echo "$f $t1 $t2 - missing!"
      re=" $f $t2 $t1 -> "
      grep -q "$re" $file || echo "$f $t2 $t1 - missing!"
    done
  done
done

@skirpichev skirpichev self-requested a review December 3, 2025 00:20
Copy link
Contributor

@skirpichev skirpichev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I've checked that special values are tested, though appreciate a second look. Please adjust PR description (an maybe title).

@yoney
Copy link
Contributor Author

yoney commented Dec 3, 2025

I've checked that special values are tested, though appreciate a second look. Please adjust PR description (an maybe title).

Thank you so much, @skirpichev

I’ve checked them as well and they look good to me. I am just sharing how I verified this locally, since it’s easy to accidentally copy-paste something incorrectly and not notice it: I compared the statically initialized version with the previous version before deleting the old tables from the code.

Copy link
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@colesbury colesbury added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 3, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @colesbury for commit 64254ba 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F142161%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 3, 2025
@colesbury
Copy link
Contributor

Buildbot failures are unrelated

@colesbury colesbury merged commit 2dac9e6 into python:main Dec 4, 2025
146 of 162 checks passed
@colesbury colesbury added the needs backport to 3.14 bugs and security fixes label Dec 4, 2025
@miss-islington-app
Copy link

Thanks @yoney for the PR, and @colesbury for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 4, 2025
…ule (pythongh-142161)

The initialization during `mod_exec` wasn't thread-safe with multiple interpreters.
(cherry picked from commit 2dac9e6)

Co-authored-by: Alper <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Dec 4, 2025

GH-142261 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Dec 4, 2025
colesbury pushed a commit that referenced this pull request Dec 4, 2025
…dule (gh-142161) (gh-142261)

The initialization during `mod_exec` wasn't thread-safe with multiple interpreters.
(cherry picked from commit 2dac9e6)

Co-authored-by: Alper <[email protected]>
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
…ule (pythongh-142161)

The initialization during `mod_exec` wasn't thread-safe with multiple interpreters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants