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

test_lzma: test_refleaks_in_decompressor___init__() leaks 100 handles on Windows #78097

Closed
vstinner opened this issue Jun 20, 2018 · 10 comments
Closed
Labels
3.7 (EOL) end of life 3.8 only security fixes OS-windows performance Performance or resource usage tests Tests in the Lib/test dir

Comments

@vstinner
Copy link
Member

BPO 33916
Nosy @pfmoore, @vstinner, @tjguk, @methane, @zware, @serhiy-storchaka, @zooba, @ZackerySpytz, @miss-islington
PRs
  • gh-67413: Fix segfaults and multiple leaks in the lzma and bz2 modules #7822
  • bpo-33916: Fix bz2 and lzma init when called twice #7843
  • [3.7] bpo-33916: Fix bz2 and lzma init when called twice (GH-7843) #7871
  • [3.6] bpo-33916: Fix bz2 and lzma init when called twice (GH-7843) #7872
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2018-06-23.13:07:46.389>
    created_at = <Date 2018-06-20.15:59:40.375>
    labels = ['3.8', '3.7', 'tests', 'OS-windows', 'performance']
    title = 'test_lzma: test_refleaks_in_decompressor___init__() leaks 100 handles on Windows'
    updated_at = <Date 2018-06-23.13:07:46.388>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2018-06-23.13:07:46.388>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-06-23.13:07:46.389>
    closer = 'vstinner'
    components = ['Tests', 'Windows']
    creation = <Date 2018-06-20.15:59:40.375>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 33916
    keywords = ['patch']
    message_count = 10.0
    messages = ['320087', '320091', '320093', '320094', '320095', '320096', '320300', '320301', '320306', '320307']
    nosy_count = 9.0
    nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'methane', 'zach.ware', 'serhiy.storchaka', 'steve.dower', 'ZackerySpytz', 'miss-islington']
    pr_nums = ['7822', '7843', '7871', '7872']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'resource usage'
    url = 'https://bugs.python.org/issue33916'
    versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

    @vstinner
    Copy link
    Member Author

    Using my PR 7827, I saw that test_refleaks_in_decompressor___init__() of test_lzma leaks 100 handles on Windows.

    Extract of the code:

        @support.refcount_test
        def test_refleaks_in_decompressor___init__(self):
            gettotalrefcount = support.get_attribute(sys, 'gettotalrefcount')
            lzd = LZMADecompressor()
            refs_before = gettotalrefcount()
            for i in range(100):
                lzd.__init__()
            self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10)

    The test comes from bpo-31787: commit d019bc8.

    @vstinner vstinner added 3.8 only security fixes OS-windows performance Performance or resource usage labels Jun 20, 2018
    @methane
    Copy link
    Member

    methane commented Jun 20, 2018

    In _lzmamodule.c

    self->lock = PyThread_allocate_lock();
    

    This seems leak some resource. But I'm not sure this lock contains refcnt on Windows.

    @ZackerySpytz
    Copy link
    Mannequin

    ZackerySpytz mannequin commented Jun 20, 2018

    Have a look at bpo-23224 and PR 7822.

    @vstinner
    Copy link
    Member Author

    This seems leak some resource. But I'm not sure this lock contains refcnt on Windows.

    PR 7827 checks for leaks of Windows handles, this issue is about Windows handles, not Python reference leaks. But a Windows lock may use a Windows handle internally, I don't know :-)

    @vstinner
    Copy link
    Member Author

    It seems like test_bz2 has the same issue, but I'm not sure:
    #7827 (comment)

    @serhiy-storchaka serhiy-storchaka added tests Tests in the Lib/test dir 3.7 (EOL) end of life labels Jun 20, 2018
    @serhiy-storchaka
    Copy link
    Member

    I four times tried to update this issue, and only the forth one was successful. Other attempts was failed because "Edit Error: someone else has edited this issue (nosy, components, versions)." This is an extreme case of race condition in real life.

    @vstinner
    Copy link
    Member Author

    New changeset 9b7cf75 by Victor Stinner in branch 'master':
    bpo-33916: Fix bz2 and lzma init when called twice (GH-7843)
    9b7cf75

    @miss-islington
    Copy link
    Contributor

    New changeset efc6bf6 by Miss Islington (bot) in branch '3.7':
    bpo-33916: Fix bz2 and lzma init when called twice (GH-7843)
    efc6bf6

    @vstinner
    Copy link
    Member Author

    New changeset 1094891 by Victor Stinner in branch '3.6':
    bpo-33916: Fix bz2 and lzma init when called twice (GH-7843) (GH-7872)
    1094891

    @vstinner
    Copy link
    Member Author

    It seems like Python 2.7 is inaffected by the bug:

    static int
    BZ2File_init(BZ2FileObject *self, PyObject *args, PyObject *kwargs)
    {
        ...
    #ifdef WITH_THREAD
        if (!self->lock) {
            self->lock = PyThread_allocate_lock();
        }
    
        if (!self->lock) {
            PyErr_SetString(PyExc_MemoryError, "unable to allocate lock");
            goto error;
        }
    #endif
        ...
    }

    I fixed the leak in Python 3.6, 3.7 and master. I close the issue.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes OS-windows performance Performance or resource usage tests Tests in the Lib/test dir
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants