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-112536: Add --tsan test for reasonable TSAN execution times. by corona10 · Pull Request #116601 · python/cpython

Conversation

@corona10

@corona10 corona10 commented Mar 11, 2024 •
edited by github-actions Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member
  • Issue: Add support for thread sanitizer (TSAN) via --with-thread-sanitizer #112536

📚 Documentation preview 📚: https://cpython-previews--116601.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

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

All reactions
corona10 added 2 commits March 11, 2024 23:13
@corona10
pythongh-112536: Add --tsan test for reasonable TSAN execution times.
60e7b12
@corona10
Update documentation
a0c5961
@corona10
corona10 requested review from colesbury and erlend-aasland March 11, 2024 14:21
@bedevere-app bedevere-app Bot added the awaiting core review label Mar 11, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Mar 11, 2024
Add support for thread sanitizer (TSAN) via --with-thread-sanitizer #112536
Closed
corona10
corona10 commented Mar 11, 2024
View reviewed changes
Comment thread Tools/tsan/supressions.txt
@@ -0,0 +1,3 @@
## reference: https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions

@corona10 corona10 Mar 11, 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.

I am going to update the list of supressions.txt when working on the CI things.

Sorry, something went wrong.

Uh oh!

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

All reactions
@corona10
fix
4230c92
corona10
corona10 commented Mar 11, 2024
View reviewed changes
Comment thread Lib/test/libregrtest/main.py Resolved

Uh oh!

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

@corona10 corona10 added the topic-free-threading label Mar 11, 2024
corona10
corona10 commented Mar 11, 2024
View reviewed changes
Comment thread Lib/test/libregrtest/tsan.py Outdated
'test_capi',
'test_code',
'test_compileall',
'test_concurrent_futures',

@corona10 corona10 Mar 11, 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.

==14799==ThreadSanitizer: starting new threads after multi-threaded fork is not supported. Dying (set die_after_fork=0 to override)

Sorry, something went wrong.

Uh oh!

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

All reactions
corona10
corona10 commented Mar 11, 2024
View reviewed changes
Comment thread Lib/test/libregrtest/tsan.py Resolved

Uh oh!

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

@corona10

corona10 commented Mar 11, 2024

Copy link
Copy Markdown
Member Author

@colesbury
We may need to decide which tests should be included.

  • Successful tests only?
  • Add failed tests too but update supressions.txt for this?
All reactions

Sorry, something went wrong.

Uh oh!

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

@corona10
Update
a13ecb7
AlexWaygood
AlexWaygood reviewed Mar 11, 2024
View reviewed changes
Comment thread Lib/test/libregrtest/main.py Resolved

Uh oh!

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

@corona10 @AlexWaygood
Update Lib/test/libregrtest/main.py
9a4b5a3
Co-authored-by: Alex Waygood <[email protected]>
@corona10

corona10 commented Mar 11, 2024

Copy link
Copy Markdown
Member Author

We may need to decide which tests should be included.

Anyway, we should exclude some of test from the list since it takes over 1 hours :)

colesbury reacted with eyes emoji
All reactions
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

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

corona10 added 4 commits March 12, 2024 00:54
@corona10
lint
2df3502
@corona10
Exclude some tests
b3fac0b
@corona10
Adjust more tests
d8d2e8b
@corona10
Adjust tests
06607e5
@corona10

corona10 commented Mar 11, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

@colesbury @swtaarrs

I think that this setting could be the minimum setup for adding TSAN in CI.
We can add more tests if needed. Here is the local execution result from my Azure VM Ubuntu.
(Standard D4s v3, ubuntu 22.04)
And I recommend not using --with-pydebug. It's super slow, but let's see if it will be an acceptable execution time on GA.
(With separated PR)

$ ./python -m test --tsan --timeout=1200
Using random seed: 1515375220
0:00:00 load avg: 0.79 Run 17 tests in parallel using 6 worker processes (timeout: 20 min, worker timeout: 25 min)
0:00:30 load avg: 2.84 running (6): test_code (30.1 sec), test_enum (30.1 sec), test_functools (30.1 sec), test_httpservers (30.1 sec), test_imaplib (30.0 sec), test_importlib (30.0 sec)
0:00:38 load avg: 3.33 [ 1/17] test_code passed (38.4 sec) -- running (5): test_enum (38.4 sec), test_functools (38.4 sec), test_httpservers (38.4 sec), test_imaplib (38.4 sec), test_importlib (38.4 sec)
0:00:57 load avg: 3.91 [ 2/17] test_functools passed (57.4 sec) -- running (4): test_enum (57.4 sec), test_httpservers (57.4 sec), test_imaplib (57.4 sec), test_importlib (57.4 sec)
0:01:05 load avg: 4.23 [ 3/17] test_imaplib passed (1 min 5 sec) -- running (3): test_enum (1 min 5 sec), test_httpservers (1 min 5 sec), test_importlib (1 min 5 sec)
0:01:36 load avg: 5.06 running (6): test_io (57.2 sec), test_enum (1 min 35 sec), test_logging (38.2 sec), test_httpservers (1 min 35 sec), test_tuple (30.1 sec), test_importlib (1 min 35 sec)
0:01:43 load avg: 5.28 [ 4/17] test_httpservers passed (1 min 43 sec) -- running (5): test_io (1 min 4 sec), test_enum (1 min 43 sec), test_logging (45.8 sec), test_tuple (37.7 sec), test_importlib (1 min 43 sec)
0:02:13 load avg: 5.58 running (6): test_io (1 min 34 sec), test_enum (2 min 13 sec), test_logging (1 min 15 sec), test_list (30.0 sec), test_tuple (1 min 7 sec), test_importlib (2 min 13 sec)
0:02:43 load avg: 5.81 running (6): test_io (2 min 4 sec), test_enum (2 min 43 sec), test_logging (1 min 45 sec), test_list (1 min), test_tuple (1 min 37 sec), test_importlib (2 min 43 sec)
0:03:13 load avg: 5.94 running (6): test_io (2 min 34 sec), test_enum (3 min 13 sec), test_logging (2 min 15 sec), test_list (1 min 30 sec), test_tuple (2 min 7 sec), test_importlib (3 min 13 sec)
0:03:13 load avg: 5.94 [ 5/17] test_tuple passed (2 min 8 sec) -- running (5): test_io (2 min 35 sec), test_enum (3 min 13 sec), test_logging (2 min 16 sec), test_list (1 min 30 sec), test_importlib (3 min 13 sec)
0:03:23 load avg: 5.95 [ 6/17] test_list passed (1 min 39 sec) -- running (4): test_io (2 min 44 sec), test_enum (3 min 22 sec), test_logging (2 min 25 sec), test_importlib (3 min 22 sec)
0:03:53 load avg: 5.95 running (6): test_io (3 min 14 sec), test_enum (3 min 52 sec), test_logging (2 min 55 sec), test_ssl (30.0 sec), test_dict (39.1 sec), test_importlib (3 min 52 sec)
0:04:03 load avg: 6.04 [ 7/17] test_logging passed (3 min 5 sec) -- running (5): test_io (3 min 24 sec), test_enum (4 min 2 sec), test_ssl (40.1 sec), test_dict (49.2 sec), test_importlib (4 min 2 sec)
/home/corona10/cpython/Lib/test/test_logging.py:783: DeprecationWarning: This process (pid=63080) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = os.fork()
/home/corona10/cpython/Lib/multiprocessing/popen_fork.py:66: DeprecationWarning: This process (pid=63080) is multi-threaded, use of fork() may lead to deadlocks in the child.
  self.pid = os.fork()
0:04:16 load avg: 5.96 [ 8/17] test_dict passed (1 min 2 sec) -- running (4): test_io (3 min 37 sec), test_enum (4 min 16 sec), test_ssl (53.7 sec), test_importlib (4 min 16 sec)
0:04:39 load avg: 6.41 [ 9/17] test_syslog passed (35.9 sec) -- running (4): test_io (4 min), test_enum (4 min 38 sec), test_ssl (1 min 15 sec), test_importlib (4 min 38 sec)
0:04:57 load avg: 6.47 [10/17] test_thread passed (40.5 sec) -- running (4): test_io (4 min 18 sec), test_enum (4 min 56 sec), test_ssl (1 min 34 sec), test_importlib (4 min 56 sec)
0:05:06 load avg: 6.56 [11/17] test_threadedtempfile passed -- running (4): test_io (4 min 27 sec), test_enum (5 min 6 sec), test_ssl (1 min 43 sec), test_importlib (5 min 6 sec)
0:05:32 load avg: 6.49 [12/17] test_ssl passed (2 min 9 sec) -- running (4): test_io (4 min 53 sec), test_enum (5 min 32 sec), test_threading_local (35.6 sec), test_importlib (5 min 32 sec)
0:05:36 load avg: 6.37 [13/17] test_threadsignals passed -- running (4): test_io (4 min 57 sec), test_enum (5 min 35 sec), test_threading_local (38.9 sec), test_importlib (5 min 35 sec)
0:05:50 load avg: 6.07 [14/17] test_enum passed (5 min 49 sec) -- running (3): test_io (5 min 11 sec), test_threading_local (53.2 sec), test_importlib (5 min 50 sec)
0:05:50 load avg: 6.07 [15/17] test_threading_local passed (53.3 sec) -- running (2): test_io (5 min 11 sec), test_importlib (5 min 50 sec)
0:06:20 load avg: 4.60 running (2): test_io (5 min 41 sec), test_importlib (6 min 20 sec)
0:06:50 load avg: 4.19 running (2): test_io (6 min 11 sec), test_importlib (6 min 50 sec)
0:07:20 load avg: 3.81 running (2): test_io (6 min 41 sec), test_importlib (7 min 20 sec)
0:07:50 load avg: 3.27 running (2): test_io (7 min 11 sec), test_importlib (7 min 50 sec)
0:08:07 load avg: 2.99 [16/17] test_importlib passed (8 min 7 sec) -- running (1): test_io (7 min 28 sec)
0:08:37 load avg: 2.20 running (1): test_io (7 min 58 sec)
0:09:07 load avg: 1.73 running (1): test_io (8 min 28 sec)
0:09:37 load avg: 1.52 running (1): test_io (8 min 58 sec)
0:09:47 load avg: 1.36 [17/17] test_io passed (9 min 8 sec)

== Tests result: SUCCESS ==

All 17 tests OK.

Total duration: 9 min 47 sec
Total tests: run=4,167 skipped=59
Total test files: run=17/17
Result: SUCCESS
All reactions

Sorry, something went wrong.

Uh oh!

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

@corona10
corona10 requested a review from swtaarrs March 11, 2024 17:04
swtaarrs
swtaarrs reviewed Mar 12, 2024
View reviewed changes
Comment thread Doc/library/test.rst Resolved

Uh oh!

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

Comment thread Lib/test/libregrtest/main.py Resolved

Uh oh!

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

Comment thread Lib/test/libregrtest/cmdline.py Outdated
group.add_argument('--pgo-extended', action='store_true',
help='enable extended PGO training (slower training)')
group.add_argument('--tsan', dest='tsan', action='store_true',
help='enable TSAN test')

@swtaarrs swtaarrs Mar 12, 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.

Can you explain what "TSAN test" means here? From the rest of the diff it sounds like it's "Tests that pass in a reasonable amount of time with TSAN enabled", but that's not clear from this help string.

Sorry, something went wrong.

Uh oh!

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

All reactions
colesbury
colesbury reviewed Mar 12, 2024
View reviewed changes
Comment thread Doc/library/test.rst Resolved

Uh oh!

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

Comment thread Lib/test/libregrtest/tsan.py Resolved

Uh oh!

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

Comment thread Lib/test/libregrtest/tsan.py Outdated
'test_importlib',
'test_io',
'test_logging',
'test_tuple',

@colesbury colesbury Mar 12, 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.

I don't think test_tuple uses threads. I don't think it's worthwhile to run tests that don't use threads, because we won't uncover anything new with TSAN.

Sorry, something went wrong.

Uh oh!

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

All reactions

@corona10 corona10 Mar 12, 2024 •
edited
Loading

Uh oh!

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

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.

I got it; I removed more tests that are not using the thread currently.

Sorry, something went wrong.

Uh oh!

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

All reactions
corona10 and others added 4 commits March 13, 2024 07:36
@corona10
Address code review
0f1899e
@corona10 @colesbury
Update Lib/test/libregrtest/tsan.py
ff8c92e
Co-authored-by: Sam Gross <[email protected]>
@corona10
Address code review
b571b9d
@corona10
Merge remote-tracking branch 'upstream/main' into pythongh-112536-tsa…
8f28847
…n-tests
@corona10 corona10 closed this Mar 12, 2024
@corona10 corona10 reopened this Mar 12, 2024
corona10
corona10 commented Mar 12, 2024
View reviewed changes
Comment thread Lib/test/libregrtest/cmdline.py
group.add_argument('--pgo-extended', action='store_true',
help='enable extended PGO training (slower training)')
group.add_argument('--tsan', dest='tsan', action='store_true',
help='run a subset of test cases that are proper for the TSAN test')

@corona10 corona10 Mar 12, 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.

@swtaarrs It will be the more clear?

Sorry, something went wrong.

Uh oh!

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

All reactions

@swtaarrs swtaarrs Apr 4, 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.

Sorry, I completely missed this notification. Looks good to me :)

Sorry, something went wrong.

Uh oh!

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

All reactions
@corona10
corona10 requested review from colesbury and swtaarrs March 12, 2024 23:22
@erlend-aasland
erlend-aasland removed their request for review March 13, 2024 23:33
colesbury
colesbury approved these changes Mar 15, 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.

This looks like a good start to me. We can build on it (edit the list of tests and suppressions) in the future.

Sorry, something went wrong.

Uh oh!

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

All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Mar 15, 2024
@corona10
Merge branch 'main' into pythongh-112536-tsan-tests
3f265e1
@corona10
corona10 merged commit ebf29b3 into python:main Mar 15, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Mar 15, 2024
@corona10
corona10 deleted the gh-112536-tsan-tests branch March 15, 2024 16:07
pitrou pushed a commit to pitrou/cpython that referenced this pull request Mar 17, 2024
@corona10 @pitrou
[3.12] pythongh-112536: Add --tsan test for reasonable TSAN execution…
3db7958
… times. (pythongh-116601)

(cherry picked from commit ebf29b3)

Co-authored-by: Donghee Na <[email protected]>
@bedevere-app

bedevere-app Bot commented Mar 17, 2024

Copy link
Copy Markdown

GH-116929 is a backport of this pull request to the 3.12 branch.

All reactions

Sorry, something went wrong.

Uh oh!

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

pitrou added a commit that referenced this pull request Mar 18, 2024
@pitrou @corona10
[3.12] gh-112536: Add --tsan test for reasonable TSAN execution times. (
fcb2301
gh-116601) (#116929)

(cherry picked from commit ebf29b3)

Co-authored-by: Donghee Na <[email protected]>
vstinner pushed a commit to vstinner/cpython that referenced this pull request Mar 20, 2024
@corona10 @vstinner
pythongh-112536: Add --tsan test for reasonable TSAN execution times. (…
66f349d
…pythongh-116601)
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@corona10 @adorilson
pythongh-112536: Add --tsan test for reasonable TSAN execution times. (…
4e68e5c
…pythongh-116601)
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@corona10 @diegorusso
pythongh-112536: Add --tsan test for reasonable TSAN execution times. (…
648dc16
…pythongh-116601)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood left review comments
@colesbury colesbury colesbury approved these changes
@swtaarrs swtaarrs Awaiting requested review from swtaarrs

Uh oh!

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

Assignees

No one assigned

Labels

topic-free-threading

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

@corona10 @swtaarrs @colesbury @AlexWaygood
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 01402520 — 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.