Issue 3660: reference leaks in test_distutils (original) (raw)
Created on 2008-08-24 19:12 by nnorwitz, last changed 2022-04-11 14:56 by admin. This issue is now closed.
Messages (27)
Author: Neal Norwitz (nnorwitz) * 
Date: 2008-08-24 19:12
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
Author: Antoine Pitrou (pitrou) *
Date: 2008-08-27 21:55
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
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *
Date: 2008-08-28 09:55
the "test_site leaked [88, 88]" is the same as problem as .
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.
Author: Antoine Pitrou (pitrou) *
Date: 2008-08-28 10:14
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...
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *
Date: 2008-08-28 11:03
tracks the memoryview issues.
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *
Date: 2008-08-28 11:30
the leaks in test_pickle and test_pickletools are corrected by the attached patch.
Author: Antoine Pitrou (pitrou) *
Date: 2008-08-29 19:09
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.
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *
Date: 2008-08-29 19:33
Oops, you are right of course. I remove my patch so we don't get confused.
Author: Antoine Pitrou (pitrou) *
Date: 2008-08-29 19:58
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:
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *
Date: 2008-08-29 20:20
Did you look at the patch for ? it should at least correct test_site.
Author: Antoine Pitrou (pitrou) *
Date: 2008-08-29 21:24
Ok, after the two patches plus the patch in #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
Author: Antoine Pitrou (pitrou) *
Date: 2008-09-05 00:04
The patch for _pickle has been committed in r66227.
Author: Antoine Pitrou (pitrou) *
Date: 2008-09-05 00:44
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
Author: Neal Norwitz (nnorwitz) *
Date: 2008-09-05 02:56
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.
Author: Antoine Pitrou (pitrou) *
Date: 2008-09-05 10:32
FWIW, applying encode-leak2.patch removes the leak in test_unicode.
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *
Date: 2008-09-05 13:08
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.
Author: Antoine Pitrou (pitrou) *
Date: 2008-09-05 13:30
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!
Author: Antoine Pitrou (pitrou) *
Date: 2008-09-05 13:49
Amaury, your patch is much clearer indeed and it fixes the leak.
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *
Date: 2008-09-05 21:00
encode-leak3.patch applied in r66234.
Author: Antoine Pitrou (pitrou) *
Date: 2008-09-05 21:47
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.
Author: Benjamin Peterson (benjamin.peterson) *
Date: 2008-09-05 21:59
test_distutils is also leaking the the trunk:
test_distutils leaked [144, 144, 144, 144] references, sum=576
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *
Date: 2008-09-05 22:19
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.
Author: Antoine Pitrou (pitrou) *
Date: 2008-09-05 22:29
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.
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *
Date: 2008-09-05 22:34
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?
Author: Antoine Pitrou (pitrou) *
Date: 2008-09-05 22:35
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.
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *
Date: 2008-09-05 22:48
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.
Author: Ezio Melotti (ezio.melotti) *
Date: 2010-01-06 22:19
This issue has been fixed.
History
Date
User
Action
Args
2022-04-11 14:56:38
admin
set
github: 47910
2010-01-06 22:19:07
ezio.melotti
set
status: open -> closed
nosy: + ezio.melotti
messages: +
resolution: fixed
stage: resolved
2008-09-05 22:48:11
amaury.forgeotdarc
set
messages: +
2008-09-05 22:35:53
pitrou
set
messages: +
2008-09-05 22:34:00
amaury.forgeotdarc
set
messages: +
2008-09-05 22:29:39
pitrou
set
messages: +
2008-09-05 22:19:45
amaury.forgeotdarc
set
messages: +
2008-09-05 21:59:15
benjamin.peterson
set
nosy: + benjamin.peterson
title: reference leaks in 3.0 -> reference leaks in test_distutils
messages: +
versions: + Python 2.6
2008-09-05 21:47:47
pitrou
set
priority: release blocker -> high
messages: +
2008-09-05 21:00:20
amaury.forgeotdarc
set
messages: +
2008-09-05 13:49:18
pitrou
set
keywords: - needs review
messages: +
2008-09-05 13:30:23
pitrou
set
messages: +
2008-09-05 13:08:33
amaury.forgeotdarc
set
files: + encode-leak3.patch
messages: +
2008-09-05 10:32:15
pitrou
set
messages: +
2008-09-05 02:56:15
nnorwitz
set
messages: +
2008-09-05 00:44:57
pitrou
set
messages: +
2008-09-05 00:04:01
pitrou
set
messages: +
2008-09-02 18:00:24
pitrou
set
keywords: + needs review
2008-08-29 21:24:13
pitrou
set
messages: +
2008-08-29 20:20:40
amaury.forgeotdarc
set
messages: +
2008-08-29 19:58:39
pitrou
set
messages: +
2008-08-29 19:33:23
amaury.forgeotdarc
set
files: - encode-leak.patch
2008-08-29 19:33:17
amaury.forgeotdarc
set
messages: +
2008-08-29 19:09:01
pitrou
set
files: + encode-leak2.patch
messages: +
2008-08-28 11:30:56
amaury.forgeotdarc
set
files: + pickle-leak.patch
messages: +
2008-08-28 11:03:09
amaury.forgeotdarc
set
messages: +
2008-08-28 10:14:32
pitrou
set
messages: +
2008-08-28 09:55:31
amaury.forgeotdarc
set
files: + encode-leak.patch
nosy: + amaury.forgeotdarc
messages: +
keywords: + patch
2008-08-27 21:55:55
pitrou
set
nosy: + pitrou
messages: +
2008-08-24 19:12:58
nnorwitz
create