-
-
Notifications
You must be signed in to change notification settings - Fork 33.3k
gh-116738: Make mmap module thread-safe #139237
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
Conversation
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 think that it would be helpful in the long run to switch mmap to AC rather than manually adding lock_held functions everywhere.
|
Same question from my side: |
@ZeroIntensity, @corona10: Thank you both for the reviews, I agree that using AC with I wanted to ask for your preference: would you like to implement the switch in this PR, or should I hold off on this one and do the implementation separately, then continue with this PR after that? Alternatively, the switch could be done as a follow-up PR. Thanks! |
|
Let's do AC in this PR. |
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 reviewed mmapmodule.c, but I didn't review Lib/test/test_free_threading/test_mmap.py.
|
🤖 New build scheduled with the buildbot fleet by @kumaraditya303 for commit 9d6b95d 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F139237%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
|
Thanks @yoney for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
|
Sorry, @yoney and @kumaraditya303, I could not cleanly backport this to |
(cherry picked from commit 7f155f9) Co-authored-by: Alper <[email protected]>
|
GH-139825 is a backport of this pull request to the 3.14 branch. |
|
…) (python#139825) * [3.14] pythongh-116738: make `mmap` module thread-safe (pythonGH-139237) (cherry picked from commit 7f155f9) Co-authored-by: Alper <[email protected]>
These changes make the
mmapmodule thread-safe for FT-Python by protecting themmap_objectagainst race conditions and undefined behavior. The goal is to provide behavior similar to standard Python builds with the GIL enabled, rather than makingmmapfully deterministic or generally recommended for multithreaded use.cc: @mpage @colesbury @Yhg1s