gh-133982: Test _pyio.BytesIO in free-threaded tests #136218
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. |
cmaloney
changed the title
|
cc @corona10 @kumaraditya303 (related to the problems in gh-135410). |
There was a problem hiding this comment.
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 |
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.