-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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: Log TSAN warnings to separate files and archive them #118747
Conversation
This ensures we don't lose races that occur in subprocesses or interleave races from workers running in parallel.
Should we |
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. |
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. |
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.
@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. |
Great! We probably want to backport this to 3.13, right? |
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=0
change 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.