gh-133982: Test _pyio.BytesIO in free-threaded tests - #136218
Conversation
Sorry, something went wrong.
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 |
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
cc @corona10 @kumaraditya303 (related to the problems in gh-135410). |
Sorry, something went wrong.
corona10
left a comment
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.
Sorry, something went wrong.
|
The C implementation (_io) was made thread safe in GH-132616, the _pyio version was not updated at that time. I don't believe _pyio is in any current CPython benchmarks so this shouldn't be critical for the performance metric. 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" |
Sorry, something went wrong.
|
merged main to rerun/work around flaky test gh-136186 |
Sorry, something went wrong.
corona10
left a comment
There was a problem hiding this comment.
LGTM, yeah we don't have to think deeply about fallback implementation.
Sorry, something went wrong.
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 / _experimentalio performance down the line including in threaded contexts. For now though, goal is simple functional thread safety iterating to better.
_pyio.BytesIO has two parts to its state, _pos and _buffer that get updated independently at times (ex. seek just changes _pos) but often together (ex. write updates _pos, maybe extends _buffer, and copies data into _buffer). When updated together multiple threads simultaneously operating could cause issues, so introduced a lock self._lock to cover those cases.