Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-117657: Log TSAN warnings to separate files and archive them#118747
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
mpage commented May 7, 2024 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
This ensures we don't lose races that occur in subprocesses or interleave races from workers running in parallel.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
colesbury commented May 8, 2024
Should we |
hugovk commented May 8, 2024
The example at https://github.com/mpage/cpython/actions/runs/8993391354 is a 170 KB zip containing 33 files, so it might not be practical to cat them all. |
mpage commented May 8, 2024
I don't feel super strongly either way. On the one hand, that doesn't seem that bad? It's also with ~all the suppressions removed, so it's roughly the worst case scenario. In the common case (i.e. a new race appears), I would imagine it would be a lot less output. On the other hand, downloading a zip file to look at the reported races doesn't feel particularly onerous. |
colesbury left a comment
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.
LGTM
I might just be lazy, but having the logs available in the GitHub UI seems nice to have. We currently print the logs and I wouldn't expect them to be significantly longer with this change. If we are concerned about super long logs, we could also just head -n 1000 or some other reasonably large length.
mpage commented May 9, 2024
@hugovk@colesbury - The most recent version of this should be a happy medium and includes a few changes:
You can look at this workflow run as an example of what things look like on both success and failure. I removed suppressions from the free-threaded build so the TSAN job fails. |
colesbury commented May 10, 2024
Great! We probably want to backport this to 3.13, right? |
mpage commented May 10, 2024
Yeah, I think that's worth doing. |
Thanks @mpage for the PR, and @colesbury for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
…pythonGH-118747) This ensures we don't lose races that occur in subprocesses or interleave races from workers running in parallel. Log files are collected and packaged into a zipfile that can be downloaded from the "Artifacts" section of the workflow run. (cherry picked from commit b88889e) Co-authored-by: mpage <mpage@meta.com>
GH-118931 is a backport of this pull request to the 3.13 branch. |
GH-118747) (#118931) This ensures we don't lose races that occur in subprocesses or interleave races from workers running in parallel. Log files are collected and packaged into a zipfile that can be downloaded from the "Artifacts" section of the workflow run. (cherry picked from commit b88889e) Co-authored-by: mpage <mpage@meta.com>
…python#118747) This ensures we don't lose races that occur in subprocesses or interleave races from workers running in parallel. Log files are collected and packaged into a zipfile that can be downloaded from the "Artifacts" section of the workflow run.
This ensures we don't lose races that occur in subprocesses or interleave races from workers running in parallel.
Log files are collected and packaged into a zipfile than can be downloaded from the "Artifacts" section of the workflow run (example).
The
handle_segv=0change to the TSAN options is necessary to avoid logs like this in default builds. I don't know what's causing this, but our ASAN builds also set the option.The additional thread leak suppression was also required.