Skip to content
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

bpo-1635741: Port _lzma module to multiphase initialization #19382

Merged
merged 5 commits into from
Jun 22, 2020

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Apr 5, 2020

@corona10
Copy link
Member Author

corona10 commented Apr 5, 2020

I ran the several leak test also for subinterp.
No leak was found on this change.

class SubinterpThreadingTests(unittest.TestCase):
    def test_lzma_subinterp(self):
        code = textwrap.dedent(r"""
            import os
            from lzma import LZMACompressor, LZMADecompressor, LZMAError
            lzc = LZMACompressor()
            lzd = LZMADecompressor()
            def noop(*args):
                pass

            os.register_at_fork(after_in_child=noop)
        """)
        ret = support.run_in_subinterp(code)

@corona10
Copy link
Member Author

corona10 commented Apr 5, 2020

@shihai1991 Can you please take a look?

@corona10 corona10 requested a review from vstinner April 5, 2020 04:44
Modules/_lzmamodule.c Outdated Show resolved Hide resolved
Modules/_lzmamodule.c Outdated Show resolved Hide resolved
@corona10 corona10 force-pushed the bpo-1635741-lzma branch 3 times, most recently from 7558ffb to 525190c Compare April 5, 2020 13:03
Copy link
Member

@shihai1991 shihai1991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, LGTM.

@corona10
Copy link
Member Author

corona10 commented Apr 5, 2020

@vstinner Please take a look :)

@corona10
Copy link
Member Author

corona10 commented Apr 6, 2020

./python.exe -m test test_capi -R 3:3
0:00:00 load avg: 1.34 Run tests sequentially
0:00:00 load avg: 1.34 [1/1] test_capi
beginning 6 repetitions
123456
......
test_capi passed in 34.3 sec

== Tests result: SUCCESS ==

1 test OK.

Total duration: 34.3 sec
Tests result: SUCCESS

with patch (https://bugs.python.org/issue40149#msg365560)

./python.exe -m test test_threading -R 3:3
0:00:00 load avg: 1.84 Run tests sequentially
0:00:00 load avg: 1.84 [1/1] test_threading
beginning 6 repetitions
123456
......
test_threading passed in 1 min 11 sec

== Tests result: SUCCESS ==

1 test OK.

Total duration: 1 min 11 sec
Tests result: SUCCESS

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just two minor coding style remarks.

Modules/_lzmamodule.c Outdated Show resolved Hide resolved
Modules/_lzmamodule.c Show resolved Hide resolved
Modules/_lzmamodule.c Outdated Show resolved Hide resolved
@corona10 corona10 merged commit 1937edd into python:master Jun 22, 2020
@corona10 corona10 deleted the bpo-1635741-lzma branch June 22, 2020 15:53
fasih pushed a commit to fasih/cpython that referenced this pull request Jun 29, 2020
arun-mani-j pushed a commit to arun-mani-j/cpython that referenced this pull request Jul 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants