GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-117657: Enable TSAN check on macOS with free-threads build by aisk · Pull Request #120502 · python/cpython

Conversation

@aisk

@aisk aisk commented Jun 14, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There are some macOS only races, for example #120500, so I think maybe we should enable the test for macOS on GHA.

Another thing is that two TSAN tests will hang: #120696, so we should skip them with TSAN on macOS.

  • Issue: Make TSAN tests pass with the GIL disabled in free-threaded builds #117657

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@aisk
Enable TSAN check on macOS with free-threads build
eb3135b
@aisk
aisk requested review from ezio-melotti and hugovk as code owners June 14, 2024 13:12
@bedevere-app bedevere-app Bot added the awaiting review label Jun 14, 2024
@aisk
aisk marked this pull request as draft June 14, 2024 13:12
@bedevere-app bedevere-app Bot removed the awaiting review label Jun 14, 2024
aisk added 4 commits June 14, 2024 21:23
@aisk
fix tsan
b86782e
@aisk
skip install dependencies on macos in tsan
0fcc952
@aisk
using different binary name for tsan test on macos
3136d75
@aisk
fix typo
3cf67e8
This was referenced Jun 14, 2024
gh-117657: Add TSAN suppression for set_default_allocator_unlocked #120500
Merged
faulthandler will hang the process with a TSAN and free-thread build Python #120696
Closed
aisk and others added 2 commits June 18, 2024 22:29
@aisk
skip some tests
9301018
@aisk
Merge branch 'main' into tsan-macos
8bc1a79
@aisk
aisk marked this pull request as ready for review June 18, 2024 14:35
@bedevere-app bedevere-app Bot added the awaiting review label Jun 18, 2024
@aisk aisk changed the title Enable TSAN check on macOS with free-threads build gh-117657: Enable TSAN check on macOS with free-threads build Jun 18, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Jun 18, 2024
Make TSAN tests pass with the GIL disabled in free-threaded builds #117657
Closed
@aisk aisk added the skip news label Jun 18, 2024
hugovk
hugovk reviewed Jun 18, 2024
View reviewed changes
Comment thread .github/workflows/build.yml
options: ./configure --config-cache --disable-gil --with-thread-sanitizer --with-pydebug
suppressions_path: Tools/tsan/suppressions_free_threading.txt
tsan_logs_artifact_name: tsan-logs-free-threading
os-matrix: '["ubuntu-22.04", "macos-14"]'

@hugovk hugovk Jun 18, 2024

Copy link
Copy Markdown
Member

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.

To avoid queuing up too many jobs for the limited number of macOS builders, we'd want to use macos-14 for forks, but Cirrus for upstream builds.

See elsewhere in this file for examples, also needs changes in the reusable workflow.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
colesbury
colesbury reviewed Jun 18, 2024
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

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.

I'm ambivalent about adding another TSan job. I'm not sure it's currently worth the extra CI resources and #120500 doesn't really seem like it uncovered a new race -- it might be due to the different version of clang or just a slightly different call stack that wasn't covered by the suppressions file.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_capi/test_mem.py
Comment on lines +78 to +80
if support.check_sanitizer(thread=True) and sys.platform == 'darwin':
# See: gh-120696
raise unittest.SkipTest("this test will hang on macOS with TSAN")

@colesbury colesbury Jun 18, 2024

Copy link
Copy Markdown
Contributor

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.

If this is hanging in faulthandler, that probably means the test is crashing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@colesbury colesbury Jun 18, 2024

Copy link
Copy Markdown
Contributor

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.

Faulthandler hanging is definitely not great, but even if faulthandler is fixed I expect there will still be a problem with this test.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@aisk aisk Jun 19, 2024

Copy link
Copy Markdown
Member Author

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.

These tests are just test the mallocator without the GIL, and intented to be crashed, so I think they are ok for now?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@colesbury colesbury Jun 20, 2024

Copy link
Copy Markdown
Contributor

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.

Oh, that makes sense -- I missed that.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@aisk

aisk commented Jun 19, 2024

Copy link
Copy Markdown
Member Author

I'm ambivalent about adding another TSan job. I'm not sure it's currently worth the extra CI resources and #120500 doesn't really seem like it uncovered a new race -- it might be due to the different version of clang or just a slightly different call stack that wasn't covered by the suppressions file.

I have another idea. We can just modified the code to temporarily skip these two problematic tests and run the TSAN checks on macOS locally. What do you think?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@colesbury

colesbury commented Jun 20, 2024

Copy link
Copy Markdown
Contributor

Yeah, I think we should be able to successfully run TSan tests locally on macOS.

There still seems to be some underlying bugs that we should fix. As you pointed out above, the test (in a subprocess) is supposed to crash because it calls PyMem_Malloc without holding the GIL / not having a valid thread state (the equivalent in the free-threaded build). The test runs:

PYTHONMALLOC=debug ./python -c "import _testcapi; _testcapi.pymem_malloc_without_gil()"

(I found it easier to run that directly under gdb or lldb than running the test case)

  • On Linux in the free-threaded build, the faulthandler itself crashes in _Py_DumpExtensionModules because it's calling PyDict_Next without a valid thread state. The test still passes because it's already printed out some of the error message.
  • On macOS, the faulthandler crash leads to a hang. I'm not sure why. I thought it might be re-entrantly triggering the faulthandler, but it doesn't look like that.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aisk aisk mentioned this pull request Jun 23, 2024
gh-120696: Skip some TSAN tests on macOS that currently fail #120900
Closed
@aisk

aisk commented Jun 23, 2024

Copy link
Copy Markdown
Member Author

Hi @colesbury, I opened #120900 just to skip these two tests on macOS.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions

github-actions Bot commented Apr 17, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 17, 2026
@ambv

ambv commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Rebase review found this PR has no anchor: upstream replaced reusable-tsan.yml and the build_tsan jobs with reusable-san.yml/build-san, so this TSAN-on-macOS CI change cannot be rebased, only re-engineered. Closing to cull the number of open pull requests. If macOS TSAN coverage is still wanted, please open a new PR against the current CI layout rather than reopening this one.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ambv ambv closed this Oct 20, 2026
@aisk
aisk deleted the tsan-macos branch October 21, 2026 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colesbury colesbury colesbury left review comments
@hugovk hugovk hugovk left review comments
@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review skip news stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@aisk @colesbury @ambv @hugovk
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.