Skip to content
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

gh-117657: Add TSAN suppressions for the free-threaded build #117736

Merged
merged 4 commits into from
Apr 15, 2024
Merged

gh-117657: Add TSAN suppressions for the free-threaded build #117736

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
509dac0
Add TSAN suppressions for the free-threaded build
mpage Apr 10, 2024
1d44c87
Skip prohibitively slow tests
mpage Apr 10, 2024
8f18671
Categorize mimalloc races
mpage Apr 11, 2024
1ea8107
Reduce iteration count instead of skipping tests
mpage Apr 11, 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
2 changes: 2 additions & 0 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,7 @@ jobs:
with:
config_hash: ${{ needs.check_source.outputs.config_hash }}
options: ./configure --config-cache --with-thread-sanitizer --with-pydebug
suppressions_path: Tools/tsan/supressions.txt

build_tsan_free_threading:
name: 'Thread sanitizer (free-threading)'
Expand All @@ -501,6 +502,7 @@ jobs:
with:
config_hash: ${{ needs.check_source.outputs.config_hash }}
options: ./configure --config-cache --disable-gil --with-thread-sanitizer --with-pydebug
suppressions_path: Tools/tsan/suppressions_free_threading.txt

# CIFuzz job based on https://google.github.io/oss-fuzz/getting-started/continuous-integration/
cifuzz:
Expand Down
6 changes: 5 additions & 1 deletion .github/workflows/reusable-tsan.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ on:
options:
required: true
type: string
suppressions_path:
description: 'A repo relative path to the suppressions file'
required: true
type: string

jobs:
build_tsan_reusable:
Expand All @@ -30,7 +34,7 @@ jobs:
sudo sysctl -w vm.mmap_rnd_bits=28
- name: TSAN Option Setup
run: |
echo "TSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/Tools/tsan/supressions.txt" >> $GITHUB_ENV
echo "TSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/${{ inputs.suppressions_path }}" >> $GITHUB_ENV
echo "CC=clang" >> $GITHUB_ENV
echo "CXX=clang++" >> $GITHUB_ENV
- name: Add ccache to PATH
Expand Down
8 changes: 8 additions & 0 deletions Lib/test/test_weakref.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ def collect():
t.join()


skip_if_tsan_and_gil_disabled = unittest.skipIf(
support.check_sanitizer(thread=True) and support.Py_GIL_DISABLED,
"Test is prohibitively slow with TSAN enabled and the GIL disabled")


class ReferencesTestCase(TestBase):

def test_basic_ref(self):
Expand Down Expand Up @@ -1877,6 +1882,7 @@ def test_make_weak_keyed_dict_repr(self):
self.assertRegex(repr(dict), '<WeakKeyDictionary at 0x.*>')

@threading_helper.requires_working_threading()
@skip_if_tsan_and_gil_disabled
def test_threaded_weak_valued_setdefault(self):
d = weakref.WeakValueDictionary()
with collect_in_thread():
Expand All @@ -1886,6 +1892,7 @@ def test_threaded_weak_valued_setdefault(self):
del x

@threading_helper.requires_working_threading()
@skip_if_tsan_and_gil_disabled
def test_threaded_weak_valued_pop(self):
d = weakref.WeakValueDictionary()
with collect_in_thread():
Expand All @@ -1895,6 +1902,7 @@ def test_threaded_weak_valued_pop(self):
self.assertIsNot(x, None) # we never put None in there!

@threading_helper.requires_working_threading()
@skip_if_tsan_and_gil_disabled
def test_threaded_weak_valued_consistency(self):
# Issue #28427: old keys should not remove new values from
# WeakValueDictionary when collecting from another thread.
Expand Down
51 changes: 51 additions & 0 deletions Tools/tsan/suppressions_free_threading.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# This file contains suppressions for the free-threaded build. It contains the
# suppressions for the default build and additional suppressions needed only in
# the free-threaded build.
#
# reference: https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions

## Default build suppresssions

race:get_allocator_unlocked
race:set_allocator_unlocked
race:mi_heap_visit_pages
race:_mi_heap_delayed_free_partial

## Free-threaded suppressions

race:_add_to_weak_set
Copy link
Member

Choose a reason for hiding this comment

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

Are there GH issues open for all these suppressions?

Copy link
Contributor Author

@mpage mpage Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Not yet, working on it :) I want to provide a bit more context (i.e. sample stacktraces + repro instructions) rather than just cutting issues without context.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to start with a simple list in an omnibus issue like #117657 so that there's a single place that people can go look to track who is working on what races.

I expect many of the races to have simple fixes, but we can break off the more complex races into their own issues as needed.

The work of collecting stacktraces and repros is work that can be split up once this PR is landed and we have a way of coordinating the work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I threw together a quick script to massage the TSAN output from the tests into an index that we can use to look for stacktraces and tests that should reproduce the races.

race:_in_weak_set
race:_Py_IsImmortal
race:_Py_IsOwnedByCurrentThread
race:_PyEval_EvalFrameDefault
race:_PyFunction_SetVersion
race:_PyImport_AcquireLock
race:_PyImport_ReleaseLock
race:_PyInterpreterState_SetNotRunningMain
race:_PyInterpreterState_IsRunningMain
race:_PyObject_GC_IS_SHARED
race:_PyObject_GC_SET_SHARED
race:_PyObject_GC_TRACK
race:_PyType_HasFeature
race:_PyType_Lookup
race:assign_version_tag
race:compare_unicode_unicode
race:delitem_common
race:dictkeys_decref
race:dictkeys_incref
race:dictresize
race:gc_collect_main
race:gc_restore_tid
race:initialize_new_array
race:insertdict
race:lookup_tp_dict
race:PyMember_GetOne
race:PyMember_SetOne
race:new_reference
race:set_contains_key
race:set_inheritable
race:start_the_world
race:tstate_set_detached
race:unicode_hash
race:update_cache
race:update_cache_gil_disabled
3 changes: 2 additions & 1 deletion Tools/tsan/supressions.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
## reference: https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions
# This file contains suppressions for the default (with GIL) build.
# reference: https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions
race:get_allocator_unlocked
race:set_allocator_unlocked
race:mi_heap_visit_pages
Expand Down
Loading