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 dcfcd146f8e6fc5c2fc16a4c192a0c5f5ca8c53c
Author: Guido van Rossum <guido@python.org>
Date:   Thu Jan 31 03:40:27 2019 -0800

    bpo-35766: Merge typed_ast back into CPython (GH-11645)


See also bpo-35878.

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.

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?

The extra referenced happen here in Python-ast.c :

value = ast2obj_string(o->type_comment);
    if (!value) goto failed;
    if (_PyObject_SetAttrId(result, &PyId_type_comment, value) == -1)
        goto failed;
    Py_DECREF(value);

ast2obj_string increments it once and the setattr does it again and then there is only one Py_DECREF.

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 <report@bugs.python.org>
wrote:

>
> Pablo Galindo Salgado <pablogsal@gmail.com> added the comment:
>
> The extra referenced happen here in Python-ast.c :
>
> value = ast2obj_string(o->type_comment);
>     if (!value) goto failed;
>     if (_PyObject_SetAttrId(result, &PyId_type_comment, value) == -1)
>         goto failed;
>     Py_DECREF(value);
>
> ast2obj_string increments it once and the setattr does it again and then
> there is only one Py_DECREF.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue35879>
> _______________________________________
>

> ast2obj_string increments it once and the setattr does it again and then there is only one Py_DECREF.

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.

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
------------
❯ git --no-pager diff
diff --git a/Python/Python-ast.c b/Python/Python-ast.c
index 1a56e90bca..cb3036dbba 100644
--- a/Python/Python-ast.c
+++ b/Python/Python-ast.c
@@ -2791,6 +2791,7 @@ ast2obj_stmt(void* _o)
         if (_PyObject_SetAttrId(result, &PyId_type_comment, value) == -1)
             goto failed;
         Py_DECREF(value);
+        Py_DECREF(value);
         break;
     case AsyncFunctionDef_kind:
         result = PyType_GenericNew(AsyncFunctionDef_type, NULL, NULL);

~/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:


     def test_funcdef(self):
-        tree = self.parse(funcdef)
+        for _ in range(100000):
+            tree = self.parse(funcdef)
         self.assertEqual(tree.body[0].type_comment, "() -> int")


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
-----------------

PYTHONDUMPREFS=1 ./python.exe -m test test_type_comments |& cut -d " " -f 3 | uniq -c | sort -n
....
  85 str
  86 str
  92 str
 110 str
 416 str

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.

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 :(

I have been trying to do the same thing and likewise, no luck understanding yet how type comments are different here.

In the meantime here's the fix for the '# type: ignore' leak. https://github.com/gvanrossum/cpython/commit/ef3b9e952906aab8f5452ebcf3b9e359d35615b4

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.

New changeset d2b4c19d53f5f021fb1c7c32d48033a92ac4fe49 by Guido van Rossum in branch 'master':
bpo-35879: Fix type comment leaks (GH-11728)
https://github.com/python/cpython/commit/d2b4c19d53f5f021fb1c7c32d48033a92ac4fe49