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

reference leaks in test_distutils #47910

Closed
nnorwitz mannequin opened this issue Aug 24, 2008 · 27 comments
Closed

reference leaks in test_distutils #47910

nnorwitz mannequin opened this issue Aug 24, 2008 · 27 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@nnorwitz
Copy link
Mannequin

nnorwitz mannequin commented Aug 24, 2008

BPO 3660
Nosy @amauryfa, @pitrou, @benjaminp, @ezio-melotti
Files
  • pickle-leak.patch
  • encode-leak2.patch
  • encode-leak3.patch
  • 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 2010-01-06.22:19:07.147>
    created_at = <Date 2008-08-24.19:12:58.619>
    labels = ['interpreter-core']
    title = 'reference leaks in test_distutils'
    updated_at = <Date 2010-01-06.22:19:07.135>
    user = 'https://bugs.python.org/nnorwitz'

    bugs.python.org fields:

    activity = <Date 2010-01-06.22:19:07.135>
    actor = 'ezio.melotti'
    assignee = 'none'
    closed = True
    closed_date = <Date 2010-01-06.22:19:07.147>
    closer = 'ezio.melotti'
    components = ['Interpreter Core']
    creation = <Date 2008-08-24.19:12:58.619>
    creator = 'nnorwitz'
    dependencies = []
    files = ['11288', '11309', '11391']
    hgrepos = []
    issue_num = 3660
    keywords = ['patch']
    message_count = 27.0
    messages = ['71851', '72045', '72075', '72076', '72079', '72083', '72150', '72152', '72154', '72156', '72160', '72558', '72560', '72561', '72583', '72589', '72592', '72594', '72627', '72633', '72634', '72637', '72638', '72639', '72641', '72644', '97331']
    nosy_count = 5.0
    nosy_names = ['nnorwitz', 'amaury.forgeotdarc', 'pitrou', 'benjamin.peterson', 'ezio.melotti']
    pr_nums = []
    priority = 'high'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue3660'
    versions = ['Python 2.6', 'Python 3.0']

    @nnorwitz
    Copy link
    Mannequin Author

    nnorwitz mannequin commented Aug 24, 2008

    Even after adding the current patch in http://bugs.python.org/issue3651
    there are many reference leaks. This bug can be a placeholder for all
    the reference leaks returned from:
    ./python ./Lib/test/regrtest.py -R 3:2 -uall,-bsddb

    The current list is:

    test_unittest leaked [124, 124] references, sum=248
    test_array leaked [110, 110] references, sum=220
    test_audioop leaked [75, 75] references, sum=150
    test_binascii leaked [4, 4] references, sum=8
    test_binhex leaked [4, 4] references, sum=8
    test_codecs leaked [3, 3] references, sum=6
    test_ctypes leaked [9, 9] references, sum=18
    test_dbm leaked [194, 194] references, sum=388
    test_dbm_gnu leaked [2, 2] references, sum=4
    test_fcntl leaked [2, 2] references, sum=4
    test_file leaked [8, 8] references, sum=16
    test_fileio leaked [1, 1] references, sum=2
    test_memoryio leaked [3, 3] references, sum=6
    test_minidom leaked [5, 5] references, sum=10
    test_mmap leaked [307, 307] references, sum=614
    test_ossaudiodev leaked [2, 2] references, sum=4
    test_pickle leaked [130, 130] references, sum=260
    test_pickletools leaked [503, 503] references, sum=1006
    test_pyexpat leaked [1, 1] references, sum=2
    test_re leaked [4, 4] references, sum=8
    test_site leaked [88, 88] references, sum=176
    test_socket leaked [13, 13] references, sum=26
    test_sqlite leaked [17, 17] references, sum=34
    test_ssl leaked [82, 82] references, sum=164
    test_struct leaked [5, 5] references, sum=10
    test_unicode leaked [2, 2] references, sum=4
    test_urllib2_localnet leaked [3, 3] references, sum=6
    test_xmlrpc leaked [18, 18] references, sum=36
    test_xmlrpc_net leaked [1, 1] references, sum=2
    test_zlib leaked [10, 10] references, sum=20

    @nnorwitz nnorwitz mannequin added release-blocker interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Aug 24, 2008
    @pitrou
    Copy link
    Member

    pitrou commented Aug 27, 2008

    As of r66047, I get the following results (without "-uall", though):

    test_unittest leaked [124, 124] references, sum=248
    test_binascii leaked [1, 1] references, sum=2
    test_distutils leaked [141, 142] references, sum=283
    test_logging leaked [219, 147] references, sum=366
    test_multiprocessing leaked [0, 1] references, sum=1
    test_pickle leaked [1, 1] references, sum=2
    test_pickletools leaked [1, 1] references, sum=2
    test_popen leaked [37, 0] references, sum=37
    test_site leaked [88, 88] references, sum=176
    test_sqlite leaked [17, 17] references, sum=34
    test_unicode leaked [2, 2] references, sum=4
    test_urllib2_localnet leaked [3, 3] references, sum=6
    test_xmlrpc leaked [-84, 85] references, sum=1

    24 tests skipped:
    test_bsddb3 test_cProfile test_codecmaps_cn test_codecmaps_hk
    test_codecmaps_jp test_codecmaps_kr test_codecmaps_tw test_curses
    test_dbm_gnu test_kqueue test_nis test_normalization
    test_ossaudiodev test_pep277 test_socketserver test_startfile
    test_tcl test_timeout test_urllib2net test_urllibnet test_winreg
    test_winsound test_xmlrpc_net test_zipfile64

    @amauryfa
    Copy link
    Member

    • the "test_site leaked [88, 88]" is the same as problem as bpo-3667.

    • test_unicodes leaks in PyUnicode_AsEncodedString (attached patch), and
      also with:
      str(memoryview(b'character buffers are decoded to unicode'), 'utf-8')
      I tried another patch, but I'm not sure: I get lost between all these
      buffers... a Py_DECREF(self->view.obj) in memory_releasebuf() seems to work.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 28, 2008

    str(memoryview(b'character buffers are decoded to unicode'), 'utf-8')
    I tried another patch, but I'm not sure: I get lost between all these
    buffers... a Py_DECREF(self->view.obj) in memory_releasebuf() seems to work.

    Could you open a separate issue for the latter? Adding a DECREF in
    memory_releasebuf() isn't the right thing to do, because PyBuffer_Release()
    already does such a DECREF. I think the problem is rather in memory_getbuf(), it
    tries to take some strange shortcuts.

    Oh, and I realize that memoryobject doesn't have tp_traverse and tp_clear, which
    looks quite wrong...

    @amauryfa
    Copy link
    Member

    bpo-3712 tracks the memoryview issues.

    @amauryfa
    Copy link
    Member

    the leaks in test_pickle and test_pickletools are corrected by the
    attached patch.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 29, 2008

    Amaury, I believe the first part of encode-leak.patch is wrong, you
    should Py_DECREF the bytearray after it has been converted to bytes, not
    before. Here is an alternate patch.

    @amauryfa
    Copy link
    Member

    Oops, you are right of course.
    I remove my patch so we don't get confused.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 29, 2008

    With the two patches applied, we are now at:

    test_unittest leaked [124, 124] references, sum=248
    test_distutils leaked [141, 142] references, sum=283
    test_docxmlrpc leaked [85, -85] references, sum=0
    test_logging leaked [366, -366] references, sum=0
    test_os leaked [37, 0] references, sum=37
    test_site leaked [88, 88] references, sum=176
    test_smtplib leaked [0, 87] references, sum=87
    test_sqlite leaked [17, 17] references, sum=34
    test_telnetlib leaked [151, -151] references, sum=0
    test_unicode leaked [1, 1] references, sum=2
    test_urllib2_localnet leaked [3, 3] references, sum=6
    test_xmlrpc leaked [-85, 0] references, sum=-85

    24 tests skipped:
    test_bsddb3 test_cProfile test_codecmaps_cn test_codecmaps_hk
    test_codecmaps_jp test_codecmaps_kr test_codecmaps_tw test_curses
    test_dbm_gnu test_kqueue test_nis test_normalization
    test_ossaudiodev test_pep277 test_socketserver test_startfile
    test_tcl test_timeout test_urllib2net test_urllibnet test_winreg
    test_winsound test_xmlrpc_net test_zipfile64
    2 skips unexpected on linux2:

    @amauryfa
    Copy link
    Member

    Did you look at the patch for bpo-3667 ? it should at least correct
    test_site.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 29, 2008

    Ok, after the two patches plus the patch in bpo-3667, I get the following:

    test_asyncore leaked [84, -84] references, sum=0
    test_distutils leaked [141, 142] references, sum=283
    test_docxmlrpc leaked [-85, 0] references, sum=-85
    test_logging leaked [219, -219] references, sum=0
    test_sqlite leaked [17, 17] references, sum=34
    test_unicode leaked [1, 1] references, sum=2
    test_urllib2_localnet leaked [3, 3] references, sum=6
    test_xmlrpc leaked [-6, -79] references, sum=-85

    @pitrou
    Copy link
    Member

    pitrou commented Sep 5, 2008

    The patch for _pickle has been committed in r66227.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 5, 2008

    Numbers for the current py3k branch (without encode-leak2.patch):

    test_distutils leaked [141, 142] references, sum=283
    test_docxmlrpc leaked [-7, -85] references, sum=-92
    test_logging leaked [0, 219] references, sum=219
    test_poplib leaked [0, 84] references, sum=84
    test_sys leaked [0, 34] references, sum=34
    test_unicode leaked [1, 1] references, sum=2
    test_urllib2_localnet leaked [3, 3] references, sum=6
    test_xmlrpc leaked [192, -190] references, sum=2

    @nnorwitz
    Copy link
    Mannequin Author

    nnorwitz mannequin commented Sep 5, 2008

    The only one that is probably an issue based on Antoine's info is:

    test_unicode leaked [1, 1] references, sum=2

    I've seen test_urllib2_localnet leak 3 before. I don't know that it's
    a real leak. I'm pretty sure it is not a regression though.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 5, 2008

    FWIW, applying encode-leak2.patch removes the leak in test_unicode.

    @amauryfa
    Copy link
    Member

    amauryfa commented Sep 5, 2008

    Antoine, it seem that with encode-leak2.patch, the error path after
    PyErr_WarnEx() leaks the value of "v".

    I rewrote the whole paragraph to make it more straightforward:

    • the normal case is tested first
    • all paths end with a "return", and no goto.
    • no need to test that PyBytes_FromStringAndSize returns a PyBytes...

    I find the code much easier to check in this form, but of course this is
    a subjective POV.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 5, 2008

    Le vendredi 05 septembre 2008 à 13:08 +0000, Amaury Forgeot d'Arc a
    écrit :

    Antoine, it seem that with encode-leak2.patch, the error path after
    PyErr_WarnEx() leaks the value of "v".

    Hmm, you are right.

    I rewrote the whole paragraph to make it more straightforward:

    • the normal case is tested first
    • all paths end with a "return", and no goto.
    • no need to test that PyBytes_FromStringAndSize returns a PyBytes...

    I'll take a look!

    @pitrou
    Copy link
    Member

    pitrou commented Sep 5, 2008

    Amaury, your patch is much clearer indeed and it fixes the leak.

    @amauryfa
    Copy link
    Member

    amauryfa commented Sep 5, 2008

    encode-leak3.patch applied in r66234.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 5, 2008

    Current status:

    test_distutils leaked [141, 142] references, sum=283
    test_logging leaked [0, -219] references, sum=-219
    test_smtplib leaked [0, 87] references, sum=87

    The distutils leak should be investigated, but the overall situation is
    rather good now. The other, transient, leaks might be due to some thread
    being cleaned up too late or something.

    @benjaminp
    Copy link
    Contributor

    test_distutils is also leaking the the trunk:

    test_distutils leaked [144, 144, 144, 144] references, sum=576

    @benjaminp benjaminp changed the title reference leaks in 3.0 reference leaks in test_distutils Sep 5, 2008
    @amauryfa
    Copy link
    Member

    amauryfa commented Sep 5, 2008

    test_distutils will be difficult; the leak is around the "import xx" in
    Lib/distutils/tests/test_build_ext.py.

    And Python/import.c says:
    /* To prevent initializing an extension module more than once, we keep a
    static dictionary 'extensions' keyed [...] by filename (for dynamically
    loaded modules). A copy of the module's dictionary is stored [...]
    */

    This dictionary keeps growing with random filenames in the temp
    directory. I can't see a way to clean it.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 5, 2008

    test_distutils will be difficult; the leak is around the "import xx" in
    Lib/distutils/tests/test_build_ext.py.

    And Python/import.c says:
    /* To prevent initializing an extension module more than once, we keep a
    static dictionary 'extensions' keyed [...] by filename (for dynamically
    loaded modules). A copy of the module's dictionary is stored [...]
    */

    This dictionary keeps growing with random filenames in the temp
    directory. I can't see a way to clean it.

    If it's just that (one leaked string per each extension module import),
    I think we can live with it.

    @amauryfa
    Copy link
    Member

    amauryfa commented Sep 5, 2008

    It's not only the name, but a copy of the whole module dict just after
    import (that's why reload(sys) takes you back the
    sys.setdefaultencoding() function).

    Actually I found a (hackish) way to clean the 'extension' dict, but this
    is not enough: the static variables in xxmodule.c cannot be cleared.
    Shall we exclude this test from the leak hunter?

    @pitrou
    Copy link
    Member

    pitrou commented Sep 5, 2008

    It's not only the name, but a copy of the whole module dict just after
    import (that's why reload(sys) takes you back the
    sys.setdefaultencoding() function).

    Ow.

    Actually I found a (hackish) way to clean the 'extension' dict, but this
    is not enough: the static variables in xxmodule.c cannot be cleared.
    Shall we exclude this test from the leak hunter?

    I'd prefer not. If we hide this leak, we'll end up forgetting about its
    existence.

    @amauryfa
    Copy link
    Member

    amauryfa commented Sep 5, 2008

    With the new module structure in 3.0, it should be possible to add a
    cleanup function. It would be a good exercise; I don't know of any
    module defining such a function.

    @ezio-melotti
    Copy link
    Member

    This issue has been fixed.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants