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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
12 changes: 9 additions & 3 deletions Lib/test/test_weakref.py
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,12 @@ class MappingTestCase(TestBase):

COUNT = 10

if support.check_sanitizer(thread=True) and support.Py_GIL_DISABLED:
# Reduce iteration count to get acceptable latency
NUM_THREADED_ITERATIONS = 1000
else:
NUM_THREADED_ITERATIONS = 100000

def check_len_cycles(self, dict_type, cons):
N = 20
items = [RefCycle() for i in range(N)]
Expand Down Expand Up @@ -1880,7 +1886,7 @@ def test_make_weak_keyed_dict_repr(self):
def test_threaded_weak_valued_setdefault(self):
d = weakref.WeakValueDictionary()
with collect_in_thread():
for i in range(100000):
for i in range(self.NUM_THREADED_ITERATIONS):
x = d.setdefault(10, RefCycle())
self.assertIsNot(x, None) # we never put None in there!
del x
Expand All @@ -1889,7 +1895,7 @@ def test_threaded_weak_valued_setdefault(self):
def test_threaded_weak_valued_pop(self):
d = weakref.WeakValueDictionary()
with collect_in_thread():
for i in range(100000):
for i in range(self.NUM_THREADED_ITERATIONS):
d[10] = RefCycle()
x = d.pop(10, 10)
self.assertIsNot(x, None) # we never put None in there!
Expand All @@ -1900,7 +1906,7 @@ def test_threaded_weak_valued_consistency(self):
# WeakValueDictionary when collecting from another thread.
d = weakref.WeakValueDictionary()
with collect_in_thread():
for i in range(200000):
for i in range(2 * self.NUM_THREADED_ITERATIONS):
o = RefCycle()
d[10] = o
# o is still alive, so the dict can't be empty
Expand Down
51 changes: 51 additions & 0 deletions Tools/tsan/suppressions_free_threading.txt
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

## 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

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:_mi_heap_delayed_free_partial
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:mi_heap_visit_pages
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
5 changes: 2 additions & 3 deletions Tools/tsan/supressions.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
## 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
race:_mi_heap_delayed_free_partial
Loading