- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 33.3k
gh-116738: Use PyMutex for lzma module #140711
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,56 @@ | ||||||||||||||||||||
| import unittest | ||||||||||||||||||||
|  | ||||||||||||||||||||
| from test.support import import_helper, threading_helper | ||||||||||||||||||||
| from test.support.threading_helper import run_concurrently | ||||||||||||||||||||
|  | ||||||||||||||||||||
| lzma = import_helper.import_module("lzma") | ||||||||||||||||||||
| from lzma import LZMACompressor, LZMADecompressor | ||||||||||||||||||||
|  | ||||||||||||||||||||
| from test.test_lzma import INPUT | ||||||||||||||||||||
|  | ||||||||||||||||||||
|  | ||||||||||||||||||||
| NTHREADS = 10 | ||||||||||||||||||||
|  | ||||||||||||||||||||
|  | ||||||||||||||||||||
| @threading_helper.requires_working_threading() | ||||||||||||||||||||
| class TestLZMA(unittest.TestCase): | ||||||||||||||||||||
| def test_compressor(self): | ||||||||||||||||||||
| lzc = LZMACompressor() | ||||||||||||||||||||
|  | ||||||||||||||||||||
| # First compress() outputs LZMA header | ||||||||||||||||||||
| header = lzc.compress(INPUT) | ||||||||||||||||||||
| self.assertGreater(len(header), 0) | ||||||||||||||||||||
|  | ||||||||||||||||||||
| def worker(): | ||||||||||||||||||||
| # it should return empty bytes as it buffers data internally | ||||||||||||||||||||
| data = lzc.compress(INPUT) | ||||||||||||||||||||
| self.assertEqual(data, b"") | ||||||||||||||||||||
|  | ||||||||||||||||||||
| run_concurrently(worker_func=worker, nthreads=NTHREADS - 1) | ||||||||||||||||||||
| full_compressed = header + lzc.flush() | ||||||||||||||||||||
| decompressed = lzma.decompress(full_compressed) | ||||||||||||||||||||
| # The decompressed data should be INPUT repeated NTHREADS times | ||||||||||||||||||||
| self.assertEqual(decompressed, INPUT * NTHREADS) | ||||||||||||||||||||
|  | ||||||||||||||||||||
| def test_decompressor(self): | ||||||||||||||||||||
| chunk_size = 128 | ||||||||||||||||||||
| chunks = [bytes([ord("a") + i]) * chunk_size for i in range(NTHREADS)] | ||||||||||||||||||||
| input_data = b"".join(chunks) | ||||||||||||||||||||
| compressed = lzma.compress(input_data) | ||||||||||||||||||||
|  | ||||||||||||||||||||
| lzd = LZMADecompressor() | ||||||||||||||||||||
| output = [] | ||||||||||||||||||||
|  | ||||||||||||||||||||
| def worker(): | ||||||||||||||||||||
| data = lzd.decompress(compressed, chunk_size) | ||||||||||||||||||||
| self.assertEqual(len(data), chunk_size) | ||||||||||||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ashm-dev I agree that  There are other tests making similar assumptions. Lines 164 to 169 in ce4b0ed 
 | ||||||||||||||||||||
| output.append(data) | ||||||||||||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
 @ashm-dev  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the free-threaded build,  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there may be some misunderstanding, could you please check the list test code below? cpython/Lib/test/test_free_threading/test_list.py Lines 20 to 28 in a3ce2f7 
 @ashm-dev I’ve tried to address all your comments, but some are still unclear to me. Could you please clarify or resolve them? Thank you! | ||||||||||||||||||||
|  | ||||||||||||||||||||
| run_concurrently(worker_func=worker, nthreads=NTHREADS) | ||||||||||||||||||||
| self.assertEqual(len(output), NTHREADS) | ||||||||||||||||||||
| # Verify the expected chunks (order doesn't matter due to append race) | ||||||||||||||||||||
| self.assertEqual(set(output), set(chunks)) | ||||||||||||||||||||
|  | ||||||||||||||||||||
|  | ||||||||||||||||||||
| if __name__ == "__main__": | ||||||||||||||||||||
| unittest.main() | ||||||||||||||||||||
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.