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

Lock around usages of imaging memory arenas #8238

Merged
merged 8 commits into from
Aug 1, 2024

Conversation

lysnikolaou
Copy link
Contributor

Related to #8199.

Did some testing with threads. Thread-sanitizer is also happy after these changes.

src/libImaging/Storage.c Outdated Show resolved Hide resolved
src/_imaging.c Outdated Show resolved Hide resolved
@nulano nulano added the Free-threading PEP 703 support label Jul 16, 2024
@lysnikolaou
Copy link
Contributor Author

This is ready to be reviewed more throughly. All tests pass and tsan is happy.

@nulano
Copy link
Contributor

nulano commented Jul 19, 2024

All tests pass and tsan is happy.

Out of curiosity, how are you testing with tsan? I don't think we have it in CI (although perhaps we should add it!).

@lysnikolaou
Copy link
Contributor Author

Out of curiosity, how are you testing with tsan? I don't think we have it in CI (although perhaps we should add it!).

I added a few very very basic threaded tests, built CPython and Pillow with tsan enabled and then ran the tests. It all takes forever, cause CPython with tsan is very slow, but it works.

If your question is how I build Pillow with tsan enabled, it is really easy:

CC="clang -fsanitize=thread" python -m pip install .

@hugovk
Copy link
Member

hugovk commented Jul 31, 2024

If no other reviews or comments, I'll merge this tomorrow.

@hugovk
Copy link
Member

hugovk commented Aug 1, 2024

Thanks @lysnikolaou!

@hugovk hugovk merged commit 5517232 into python-pillow:main Aug 1, 2024
50 of 52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Free-threading PEP 703 support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants