Skip to content

gh-117657: Enable TSAN check on macOS with free-threads build #120502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+24 −3
Open

gh-117657: Enable TSAN check on macOS with free-threads build #120502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
eb3135b
Enable TSAN check on macOS with free-threads build
aisk Jun 14, 2024
b86782e
fix tsan
aisk Jun 14, 2024
0fcc952
skip install dependencies on macos in tsan
aisk Jun 14, 2024
3136d75
using different binary name for tsan test on macos
aisk Jun 14, 2024
3cf67e8
fix typo
aisk Jun 14, 2024
9301018
skip some tests
aisk Jun 18, 2024
8bc1a79
Merge branch 'main' into tsan-macos
aisk Jun 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ jobs:
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"]'
Copy link
Member

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.


# CIFuzz job based on https://google.github.io/oss-fuzz/getting-started/continuous-integration/
cifuzz:
Expand Down
16 changes: 14 additions & 2 deletions .github/workflows/reusable-tsan.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,19 @@ on:
description: 'Name of the TSAN logs artifact. Must be unique for each job.'
required: true
type: string
os-matrix:
required: false
type: string
default: '["ubuntu-22.04"]'

jobs:
build_tsan_reusable:
name: 'Thread sanitizer'
runs-on: ubuntu-22.04
name: 'Thread sanitizer (${{ matrix.os }})'
runs-on: ${{ matrix.os }}
timeout-minutes: 60
strategy:
matrix:
os: ${{fromJson(inputs.os-matrix)}}
steps:
- uses: actions/checkout@v4
- name: Runner image version
Expand All @@ -31,6 +38,7 @@ jobs:
path: config.cache
key: ${{ github.job }}-${{ runner.os }}-${{ env.IMAGE_VERSION }}-${{ inputs.config_hash }}
- name: Install Dependencies
if: ${{ startsWith(matrix.os, 'ubuntu') }}
run: |
sudo ./.github/workflows/posix-deps-apt.sh
# Install clang-18
Expand Down Expand Up @@ -64,6 +72,10 @@ jobs:
run: make pythoninfo
- name: Tests
run: ./python -m test --tsan -j4
if: ${{ startsWith(matrix.os, 'ubuntu') }}
- name: Tests
run: ./python.exe -m test --tsan -j4
if: ${{ startsWith(matrix.os, 'macos') }}
- name: Display TSAN logs
if: always()
run: find ${GITHUB_WORKSPACE} -name 'tsan_log.*' | xargs head -n 1000
Expand Down
10 changes: 9 additions & 1 deletion Lib/test/test_capi/test_mem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import re
import sys
import textwrap
import unittest


from test import support
from test.support import import_helper, requires_subprocess
from test.support.script_helper import assert_python_failure, assert_python_ok
Expand Down Expand Up @@ -75,12 +75,20 @@ def check_malloc_without_gil(self, code):
def test_pymem_malloc_without_gil(self):
# Debug hooks must raise an error if PyMem_Malloc() is called
# without holding the GIL
if support.check_sanitizer(thread=True) and sys.platform == 'darwin':
# See: gh-120696
raise unittest.SkipTest("this test will hang on macOS with TSAN")
Comment on lines +78 to +80
Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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.


code = 'import _testcapi; _testcapi.pymem_malloc_without_gil()'
self.check_malloc_without_gil(code)

def test_pyobject_malloc_without_gil(self):
# Debug hooks must raise an error if PyObject_Malloc() is called
# without holding the GIL
if support.check_sanitizer(thread=True) and sys.platform == 'darwin':
# See: gh-120696
raise unittest.SkipTest("this test will hang on macOS with TSAN")

code = 'import _testcapi; _testcapi.pyobject_malloc_without_gil()'
self.check_malloc_without_gil(code)

Expand Down
Loading