gh-116738: Use PyMutex for lzma module #140711
Conversation
| def worker(): | ||
| # it should return empty bytes as it buffers data internally | ||
| data = lzc.compress(INPUT) | ||
| self.assertEqual(data, b"") |
There was a problem hiding this comment.
The assertion self.assertEqual(data, b"") is flaky. In free-threaded mode, compress() may return data chunks non-deterministically due to race conditions in internal buffering.
There was a problem hiding this comment.
@ashm-dev Thanks for your comment. I’m trying to verify/test the mutex is protecting the internal state and buffering, so there shouldn’t be a race condition. Could you please explain which race condition you mean? That would help me understand your point better.
There was a problem hiding this comment.
@ashm-dev Are you using ChatGPT or another LLM to review for you? If so, please don't -- it's not helpful. If not, please try to be clearer in your responses.
| def worker(): | ||
| data = lzd.decompress(compressed, chunk_size) | ||
| self.assertEqual(len(data), chunk_size) | ||
| output.append(data) |
There was a problem hiding this comment.
output.append(data) without synchronization causes race conditions in free-threaded mode, potentially losing data or corrupting the list.
There was a problem hiding this comment.
output.append(data)without synchronization causes race conditions in free-threaded mode, potentially losing data or corrupting the list.
@ashm-dev list is thread safe in free-threaded build.
There was a problem hiding this comment.
In the free-threaded build, list operations use internal locks to avoid crashes, but thread safety isn’t guaranteed for concurrent mutations — see Python free-threading HOWTO.
|
|
||
| def worker(): | ||
| data = lzd.decompress(compressed, chunk_size) | ||
| self.assertEqual(len(data), chunk_size) |
There was a problem hiding this comment.
self.assertEqual(len(data), chunk_size) is wrong. decompress() may return less than max_length bytes.
There was a problem hiding this comment.
@ashm-dev I agree that decompress() can return less than max_length if there isn’t enough input. In this test, I’m providing input that should produce at least max_length bytes. Is there anything else I might be missing? If I give enough valid input, is there any reason why lzma wouldn’t return max_length?
There are other tests making similar assumptions.
Lines 164 to 169 in ce4b0ed
5 participants
Similar to #140555, the main goal was to review the
lzmamodule for free-threading. The methods already use a lock, which makes them thread-safe in a free-threaded build. I replacedPyThread_acquire_lockwithPyMutex.PyMutexreleases the GIL when the thread is parked. This change removes some macros and allocation handling code.cc: @mpage @colesbury @emmatyping