CI: update sanitizer CI to use python compiled with ASAN and TSAN #28273
Conversation
| - name: Build Python with thread sanitizer support | ||
| run: | | ||
| # free-threaded Python is much more likely to trigger races | ||
| CONFIGURE_OPTS="--with-thread-sanitizer" pyenv install 3.13t |
There was a problem hiding this comment.
This takes about 10 minutes and Python releases happen infrequently enough I think we'd benefit from using a cache. Is it ok to add a new Github actions cache entry for this step? I don't think the Python installation is all that big on-disk but I can double check.
There was a problem hiding this comment.
Only five minutes on the CI run for this PR 🤷♂️
There was a problem hiding this comment.
That seems fine to me. Not essential though if it's indeed 5 minutes - whatever you prefer.
| pip install -r requirements/test_requirements.txt | ||
| - name: Build | ||
| run: | ||
| python -m spin build -j2 -- -Db_sanitize=thread |
There was a problem hiding this comment.
this might also benefit from setting up ccache
There was a problem hiding this comment.
This looks pretty good! Moving to macOS is making the runs more expensive (latency wise, we don't actually have to pay for it), but in this particular instance there are good reasons for it.
I reviewed only the CI changes, approving that part of the PR since the jobs look fine as is, even though you may still want to add caching.
|
Once the aarch64 CI is merged I might move one of these over to using an aarch64 ubuntu image and I'll take a look at whether that improves build times. |
|
Is the fix without all the rest worth backporting? |
|
Yup, I'm actually in the middle of doing a PR. |
BUG: backport resource cleanup bugfix from gh-28273
Fixes #28267.
This PR substantially updates the sanitizer CI.
macos-latestrunner to use a CPU with relaxed memory ordering. This also more closely matches my development environment so it's more convenient for me.ufunc.atimplementation that is fixed in this PR.import threading, which cuts down on the time by quite a bit. TSAN is only useful if you actually trigger thread safety issues by doing concurrent operations, so it's not very useful to run it on single-threaded tests.