-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
gh-133982: Test _pyio.BytesIO in free-threaded tests #136218
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
The test was only checking one of the two I/O implementations. Ideally the two implementations should match.
|
The ThreadSanitizer data race failure looks real, investigating |
This requires updating pickling to exclude the lock
|
Updated to lock operations that effect multiple members which need to stay "in sync" (ex. buffer length + position in buffer during write). Believe this is ready for review. |
|
cc @corona10 @kumaraditya303 (related to the problems in gh-135410). |
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.
Is the current C implementation of BytesIO thread-safe?
I couldn’t find any mention in the documentation (https://docs.python.org/3/library/io.html#binary-i-o) explicitly stating whether BytesIO is thread-safe. If the C implementation is already thread-safe, I’d like to suggest updating the documentation to clarify that.
|
The C implementation ( Added a note about thread safety to the docs. Would be nice if there was a standard sphinx tag / annotation that could be added to objects to mark them as "safe to interact with from multiple threads in free-threaded build" |
|
merged main to rerun/work around flaky test gh-136186 |
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, yeah we don't have to think deeply about fallback implementation.
The test was only checking one of the two I/O implementations. Ideally the two implementations should match behavior (and guarantees) in free-threaded Python.
Followed https://py-free-threading.github.io/porting/#general-considerations-for-porting as a general guide for "make multi-threaded safe". I have a general project to build benchmarks around I/O in my backlog (python/pyperformance#399) where I will likely work on optimizing
_io/_pyio/_experimentalioperformance down the line including in threaded contexts. For now though, goal is simple functional thread safety iterating to better._pyio.BytesIOhas two parts to its state,_posand_bufferthat get updated independently at times (ex.seekjust changes_pos) but often together (ex.writeupdates_pos, maybe extends_buffer, and copies data into_buffer). When updated together multiple threads simultaneously operating could cause issues, so introduced a lockself._lockto cover those cases.