Skip to content

test_threading leaked [38, 38, 38] references, sum=114 #84330

Closed
Closed
test_threading leaked [38, 38, 38] references, sum=114#84330
Labels
3.9 (EOL)end of lifetestsTests in the Lib/test dir

Description

@vstinner
vstinner
opened on Apr 2, 2020
BPO 40149
Nosy @vstinner, @corona10, @shihai1991
PRs
  • bpo-40149: Implement traverse in _abc._abc_data #19412
    		•
    Files
  • test_leak.py
    		•

    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 2020-04-27.12:58:38.703>
created_at = <Date 2020-04-02.01:54:05.610>
labels = ['tests', '3.9']
title = 'test_threading leaked [38, 38, 38] references, sum=114'
updated_at = <Date 2020-04-27.12:58:38.702>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2020-04-27.12:58:38.702>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2020-04-27.12:58:38.703>
closer = 'vstinner'
components = ['Tests']
creation = <Date 2020-04-02.01:54:05.610>
creator = 'vstinner'
dependencies = []
files = ['49024']
hgrepos = []
issue_num = 40149
keywords = ['patch']
message_count = 11.0
messages = ['365557', '365559', '365560', '365566', '365695', '365705', '365912', '365913', '365915', '365938', '367415']
nosy_count = 3.0
nosy_names = ['vstinner', 'corona10', 'shihai1991']
pr_nums = ['19412']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue40149'
versions = ['Python 3.9']

    Activity

    1. vstinner commented on Apr 2, 2020

      @vstinner
      vstinner
      on Apr 2, 2020
      MemberAuthor

      New changeset 53e4c91 by Dong-hee Na in branch 'master':
      bpo-40077: Convert _abc module to use PyType_FromSpec() (GH-19202)
      53e4c91

      This change introduced a reference leak:

      $ ./python -m test -R 3:3 test_threading -m test.test_threading.SubinterpThreadingTests.test_threads_join_2
0:00:00 load avg: 1.52 Run tests sequentially
0:00:00 load avg: 1.52 [1/1] test_threading
beginning 6 repetitions
123456
......
test_threading leaked [19, 19, 19] references, sum=57
test_threading leaked [12, 12, 12] memory blocks, sum=36
test_threading failed

      == Tests result: FAILURE ==

      1 test failed:
      test_threading

      Total duration: 768 ms
      Tests result: FAILURE

    2. vstinner
      added
      3.9 (EOL)end of life
      testsTests in the Lib/test dir
      on Apr 2, 2020

    3. vstinner commented on Apr 2, 2020

      @vstinner
      vstinner
      on Apr 2, 2020
      MemberAuthor

      Attached test_leak.py is enough to reproduce the leak. It's related to subinterpreters and the _abc module.

    4. vstinner commented on Apr 2, 2020

      @vstinner
      vstinner
      on Apr 2, 2020
      MemberAuthor

      I'm not sure why, but trigger explicitly a second GC collection fix the issue.

      diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 2d5cb0ff78..d20ae01238 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -1295,6 +1295,7 @@ finalize_interp_clear(PyThreadState *tstate)
     /* Trigger a GC collection on subinterpreters*/
     if (!is_main_interp) {
         _PyGC_CollectNoFail();
+        _PyGC_CollectNoFail();
     }
 
     finalize_interp_types(tstate, is_main_interp);

    5. corona10 commented on Apr 2, 2020

      @corona10
      corona10
      on Apr 2, 2020
      Member

      FYI,
      first gc collect 772
      secondary gc collect 4

    6. corona10 commented on Apr 3, 2020

      @corona10
      corona10
      on Apr 3, 2020
      Member

      gc: collectable <type 0x7fc033c3cbd0>
      gc: collectable <dict 0x104d79830>
      gc: collectable <tuple 0x104d725a0>
      gc: collectable <builtin_function_or_method 0x104d79890>

      is not collected for the first time.

    7. corona10 commented on Apr 3, 2020

      @corona10
      corona10
      on Apr 3, 2020
      Member

      Running
      from abc import ABCMeta
      on the subinterpreter makes same size of leak.

    8. vstinner commented on Apr 7, 2020

      @vstinner
      vstinner
      on Apr 7, 2020
      MemberAuthor

      New changeset 9cc3ebd by Victor Stinner in branch 'master':
      bpo-40149: Implement traverse in _abc._abc_data (GH-19412)
      9cc3ebd

    9. vstinner commented on Apr 7, 2020

      @vstinner
      vstinner
      on Apr 7, 2020
      MemberAuthor

      In _abcmodule_exec(), when _abc_data type is created, it's created with refcnt=3:

      • 1 strong reference (normal)
      • +1 ref from tp_dict['__new__'] slot
      • +1 ref from tp_mro

      type_traverse() visits tp_dict and tp_mro, so it's fine.

      In Py_EndInterpreter(), PyInterpreterState_Clear() clears os.register_atfork() callbacks which were one of the last references to _abc_data type. The first call to _PyGC_CollectNoFail() destroys _abc_data *instances* but not the _abc_data type.

      The following patch works around the issue:

      diff --git a/Modules/_abc.c b/Modules/_abc.c
index 1efc98bf72..410dbcf96a 100644
--- a/Modules/_abc.c
+++ b/Modules/_abc.c
@@ -54,6 +54,7 @@ typedef struct {
 static int
 abc_data_traverse(_abc_data *self, visitproc visit, void *arg)
 {
+    Py_VISIT(Py_TYPE(self));
     Py_VISIT(self->_abc_registry);
     Py_VISIT(self->_abc_cache);
     Py_VISIT(self->_abc_negative_cache);
      $ ./python -m test -R 3:3 test_threading -m test.test_threading.SubinterpThreadingTests.test_threads_join_2
(...)
Tests result: SUCCESS

      I'm not sure why Py_VISIT(Py_TYPE(self)) is needed. Maybe it's a regression caused by commit 364f0b0 of bpo-35810.

      It sems like the GC doesn't take in account that instances of types allocated on the heap (if type->tp_flags has the Py_TPFLAGS_HEAPTYPE flag) hold a strong refeference to the type (PyObject.ob_type).

      I created bpo-40217: "The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type".

    10. corona10 commented on Apr 7, 2020

      @corona10
      corona10
      on Apr 7, 2020
      Member

      Wow Thank you for the summary :)

    11. vstinner commented on Apr 7, 2020

      @vstinner
      vstinner
      on Apr 7, 2020
      MemberAuthor

      bpo-40149: Implement traverse in _abc._abc_data (GH-19412)

      Pablo told me that this change is not correct: the overriden traverse function must call PyType_Type.tp_traverse (parent method).

    12. vstinner commented on Apr 27, 2020

      @vstinner
      vstinner
      on Apr 27, 2020
      MemberAuthor

      I created bpo-40217: "The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type".

      This issue is now fixed. I tested manually: I confirm that it does fix the test_threading leak, so I close the issue.

      $ ./python -m test -R 3:3 test_threading
(...)
== Tests result: SUCCESS ==

      1 test OK.

      Total duration: 1 min 14 sec
      Tests result: SUCCESS

    13. vstinner
      closed this as completedon Apr 27, 2020
    14. vstinner
      closed this as completedon Apr 27, 2020
    15. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.9 (EOL)end of lifetestsTests in the Lib/test dir

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions