Repository navigation
test_type_comments leaks references #80060
Description
| BPO | 35879 |
|---|---|
| Nosy | @gvanrossum, @ambv, @pablogsal |
| PRs |
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 = 'https://github.com/gvanrossum'
closed_at = <Date 2019-02-01.23:31:43.777>
created_at = <Date 2019-02-01.11:30:04.496>
labels = ['3.8', 'type-bug', 'library']
title = 'test_type_comments leaks references'
updated_at = <Date 2019-02-01.23:31:43.777>
user = 'https://github.com/vstinner'bugs.python.org fields:
activity = <Date 2019-02-01.23:31:43.777>
actor = 'gvanrossum'
assignee = 'gvanrossum'
closed = True
closed_date = <Date 2019-02-01.23:31:43.777>
closer = 'gvanrossum'
components = ['Library (Lib)']
creation = <Date 2019-02-01.11:30:04.496>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 35879
keywords = ['patch', 'patch', 'patch']
message_count = 13.0
messages = ['334671', '334682', '334702', '334708', '334709', '334713', '334717', '334718', '334721', '334722', '334723', '334730', '334731']
nosy_count = 3.0
nosy_names = ['gvanrossum', 'lukasz.langa', 'pablogsal']
pr_nums = ['11728', '11728', '11728']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue35879'
versions = ['Python 3.8']
Activity
vstinner commentedon Feb 1, 2019
https://buildbot.python.org/all/#/builders/1/builds/490
https://buildbot.python.org/all/#/builders/80/builds/499
test_type_comments leaked [37, 37, 37] references, sum=111
test_type_comments leaked [11, 12, 11] memory blocks, sum=34
Example:
vstinner@apu$ ./python -m test -R 3:3 test_type_comments -m test.test_type_comments.TypeCommentTests.test_asyncdef
Run tests sequentially
0:00:00 load avg: 0.56 [1/1] test_type_comments
beginning 6 repetitions
123456
......
test_type_comments leaked [2, 2, 2] references, sum=6
test_type_comments leaked [2, 3, 2] memory blocks, sum=7
test_type_comments failed
== Tests result: FAILURE ==
1 test failed:
test_type_comments
Total duration: 129 ms
Tests result: FAILURE
I bet that the regression comes from:
commit dcfcd14
Author: Guido van Rossum <[email protected]>
Date: Thu Jan 31 03:40:27 2019 -0800
See also bpo-35878.
pablogsal commentedon Feb 1, 2019
From the duplicate (https://bugs.python.org/issue35881) :
After some hours of debugging, I have identified the (possible) cause of the majority of the refleaks.
The type_comments created in ast.c with new_type_comment() never reaches 0 reference counts. This **seems** to be because the cleanup for the stmt_ty elements where the type_comments are included never call DECREF on these elements when destroyed unless I am missing something fundamental.
gvanrossum commentedon Feb 1, 2019
I can confirm it, but I don't understand it yet. Somehow the type_comment fields get an extra reference count and are never freed. How are other string fields handled?
pablogsal commentedon Feb 1, 2019
The extra referenced happen here in Python-ast.c :
ast2obj_string increments it once and the setattr does it again and then there is only one Py_DECREF.
gvanrossum commentedon Feb 1, 2019
OK, let me make a PR, I found another leak for type:ignore.
On Fri, Feb 1, 2019 at 9:22 AM Pablo Galindo Salgado <[email protected]>
wrote:
gvanrossum commentedon Feb 1, 2019
Actually I don't understand how this is the leak. Adding another Py_DECREF(value) causes an immediate crash. I've been trying to follow other paths but haven't found the root yet.
pablogsal commentedon Feb 1, 2019
Hummmm.... I may be missing something but these is what I am seeing:
BASELINE
--------
❯ ./python.exe -m test test_type_comments -R 10:10
Run tests sequentially
0:00:00 load avg: 2.30 [1/1] test_type_comments
beginning 20 repetitions
12345678901234567890
....................
test_type_comments leaked [37, 37, 37, 37, 37, 37, 37, 37, 37, 37] references, sum=370
test_type_comments leaked [11, 12, 11, 11, 11, 11, 11, 11, 11, 11] memory blocks, sum=111
test_type_comments failed
== Tests result: FAILURE ==
1 test failed:
test_type_comments
Total duration: 1 sec 28 ms
Tests result: FAILURE
EXTRA_DECREF
------------
~/github/cpython master ✗
❯ ./python.exe -m test test_type_comments -R 10:10
Run tests sequentially
0:00:00 load avg: 2.12 [1/1] test_type_comments
beginning 20 repetitions
12345678901234567890
....................
test_type_comments leaked [8, 9, 8, 8, 8, 8, 8, 8, 8, 8] memory blocks, sum=81
test_type_comments failed
== Tests result: FAILURE ==
1 test failed:
test_type_comments
Total duration: 976 ms
Tests result: FAILURE
So my interpretation is that there are still leaks somewhere or, but the extra-decref resolves the reference leaks somehow. Probably the extra DECREF here is not the proper fix, but is as far as I went when I started debugging this morning. Also, if you change one test to do the parsing in a loop:
And you ran test_type_comments.py with PYTHONDUMPREFS=1:
WITHOUT EXTRA DECREF
--------------------
65 wrapper_descriptor
75 str
82 str
85 str
86 str
92 str
110 str
416 str
200015 str
WITH EXTRA DECREF
-----------------
pablogsal commentedon Feb 1, 2019
Note:
the extra DECREF will fail in other tests because it will deallocate None if ast2obj_object returns Py_None because the comment is NULL.
pablogsal commentedon Feb 1, 2019
I'm almost sure this is not the correct fix, but I think these findings point to that object as the one being leaked. I do not understand how this object should be deallocated and where, I am trying to follow how the other PyObject* (name) is deallocated but I am not finding anything :(
gvanrossum commentedon Feb 1, 2019
I have been trying to do the same thing and likewise, no luck understanding yet how type comments are different here.
gvanrossum commentedon Feb 1, 2019
In the meantime here's the fix for the '# type: ignore' leak. gvanrossum@ef3b9e9
gvanrossum commentedon Feb 1, 2019
Found it! You need to call PyArena_AddPyObject(). I racked my brain and finally found the example code I needed at the end of new_identifier(). Please review quickly so we won't ship 3.8.0a1 with a memory leak.
gvanrossum commentedon Feb 1, 2019
New changeset d2b4c19 by Guido van Rossum in branch 'master':
bpo-35879: Fix type comment leaks (GH-11728)
d2b4c19