bpo-38152: Fix refleak in the finalizer of AST type

[eduardo-elizondo]

The AST_object was leaking its reference to the AST type. This change properly decreases the reference to the type at deallocation time. Also, this modifies tp_free to use PyType_GetSlot to make PyTypeObject opaque.

https://bugs.python.org/issue38152

[eduardo-elizondo]

Previously this was failing, and it's passing now:

eelizondo@build -> ./python -m test test_builtin -R 3:3
Run tests sequentially
0:00:00 load avg: 4.02 [1/1] test_builtin
beginning 6 repetitions
123456
......

== Tests result: SUCCESS ==

1 test OK.

Total duration: 1 sec 339 ms
Tests result: SUCCESS

I also ran all the modules that were referred to by the issue and all of them are passing now:

eelizondo@build -> ./python -m test test_ast test_builtin test_capi test_check_c_globals test_clinic test_compile test_dataclasses test_dbm test_dbm_dumb test_decimal test_fstring test_getpass test_idle test_importlib test_inspect test_pydoc test_source_encoding test_symtable test_sys test_type_comments test_types test_unittest -R 3:3 -j0
Run tests in parallel using 50 child processes
0:00:00 load avg: 8.51 [ 1/22] test_idle skipped
test_idle skipped -- No module named '_tkinter'
0:00:00 load avg: 8.51 [ 2/22] test_symtable passed
beginning 6 repetitions
123456
......
0:00:00 load avg: 8.51 [ 3/22] test_type_comments passed
beginning 6 repetitions
123456
......
0:00:00 load avg: 8.51 [ 4/22] test_check_c_globals passed
beginning 6 repetitions
123456
......
0:00:00 load avg: 8.51 [ 5/22] test_types passed
beginning 6 repetitions
123456
......
0:00:01 load avg: 8.51 [ 6/22] test_getpass passed
beginning 6 repetitions
123456
......
0:00:01 load avg: 8.51 [ 7/22] test_clinic passed
beginning 6 repetitions
123456
......
0:00:02 load avg: 8.51 [ 8/22] test_builtin passed
beginning 6 repetitions
123456
......
0:00:03 load avg: 8.51 [ 9/22] test_dataclasses passed
beginning 6 repetitions
123456
......
0:00:03 load avg: 8.51 [10/22] test_dbm passed
beginning 6 repetitions
123456
......
0:00:04 load avg: 8.51 [11/22] test_source_encoding passed
beginning 6 repetitions
123456
......
0:00:09 load avg: 9.19 [12/22] test_fstring passed
beginning 6 repetitions
123456
......
0:00:09 load avg: 9.19 [13/22] test_dbm_dumb passed
beginning 6 repetitions
123456
......
0:00:10 load avg: 9.65 [14/22] test_inspect passed
beginning 6 repetitions
123456
......
0:00:20 load avg: 10.01 [15/22] test_ast passed
beginning 6 repetitions
123456
......
0:00:21 load avg: 10.01 [16/22] test_compile passed
beginning 6 repetitions
123456
......
0:00:26 load avg: 9.85 [17/22] test_sys passed
beginning 6 repetitions
123456
......
0:00:28 load avg: 9.85 [18/22] test_pydoc passed
beginning 6 repetitions
123456
......
0:00:35 load avg: 9.10 [19/22] test_unittest passed (34 sec 759 ms) -- running: test_capi (35 sec 203 ms), test_decimal (35 sec 196 ms), test_importlib (35 sec 180 ms)
beginning 6 repetitions
123456
......
0:00:42 load avg: 8.61 [20/22] test_importlib passed (42 sec 79 ms) -- running: test_capi (42 sec 574 ms), test_decimal (42 sec 567 ms)
beginning 6 repetitions
123456
......
0:00:45 load avg: 8.56 [21/22] test_decimal passed (45 sec 291 ms) -- running: test_capi (45 sec 730 ms)
beginning 6 repetitions
123456
......
running: test_capi (1 min 15 sec)
running: test_capi (1 min 45 sec)
running: test_capi (2 min 15 sec)
running: test_capi (2 min 45 sec)
running: test_capi (3 min 15 sec)
0:03:44 load avg: 3.57 [22/22] test_capi passed (3 min 44 sec)
beginning 6 repetitions
123456
......

== Tests result: SUCCESS ==

21 tests OK.

1 test skipped:
    test_idle

Total duration: 3 min 44 sec
Tests result: SUCCESS

[eduardo-elizondo]

cc @matrixise

[ammaraskar]

LGTM with requested change.

I think a little bit of documentation change would be helpful here as well. With heap-allocated types like this, if there is a custom tp_dealloc then the user must remember to DECREF the type object.

I noticed when you introduced INCREF for heap types, you added a whatsnew entry. I think it would be advisable to add this information to https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_dealloc as well, just so there's somewhere to point to about this behavior.

I would even suggest add a comment here explaining the reason for the DECREF on the type.

[ammaraskar]

While you're in here, could you fix the typo introduced here:

cpython/Python/Python-ast.c

Lines 1236 to 1237 in d057b89

	{Py_tp_free, PyType_GenericNew},
	{Py_tp_free, PyObject_GC_Del},

(Py_tp_free is duplicated, once incorrectly set to PyType_GenericNew)

[bedevere-bot]

@DinoV: Please replace # with GH- in the commit message next time. Thanks!

[eduardo-elizondo]

@ammaraskar makes sense. I'll send a PR updating the docs