Uploaded image for project: 'Qt for Python'
  1. Qt for Python
  2. PYSIDE-939

Python 3.8 Creates Memory Leak And Negative Refcount

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P2: Important
    • Resolution: Done
    • Affects Version/s: dev
    • Fix Version/s: 5.14.0
    • Component/s: PySide, Shiboken
    • Labels:
    • Commits:
      45a3efb4e179e83f1bea69cccb190945fbc8c1f8 (pyside/pyside-setup/5.14)

      Description

      The Python developers isolated a reference count problem with heap types. They introduced a patch that corrects this, but causes memory leaks if the application code does not react accordingly. https://bugs.python.org/issue35810

      Python 3.8 is out, and we tried to build PySide with it. It was not expected that so much errors would be generated, but the creation of any class is giving negative refcounts. We expected a memory leak, only. Investigation has started on 2019-10-28.

      Update 2019-11-19:
      It turned out that the true issue concerning refcounts is really nothing special. It is the Python checkin bpo-35810: 364f0b0f19 from March 27, 2019. It is relatively easy to resolve.
      To find the real problem, we did not have any clue, and nothing else helped but a logarithmic search through the over 1000 possible checkins which caused the problem. Things got much more error prone because of the six parallel Python threads that were mixed, and we had to work through about 30 compilations (instead of the optimal 10), until the result was clear:

      It was https://bugs.python.org/issue36922 Py_TPFLAGS_METHOD_DESCRIPTOR
      (GH-13865) 988fff5d0efrom 2019-06-17T14:12 .

      Ignoring this flag has the unforeseen side-effect of a drastic reference count mismatch, which will be tackled now (PEP 590). More to be expected, soon.

      Update 2019-11-20:
      There seems to be some uncommon reference treatment that did not harm Python 3.7, but was not correct. Due to the different code paths taken by the new flag, this uncommon reference treatment becomes really wrong.
      An adjustment of reference counting in the way the original https://bugs.python.org/issue35810 wants to see things corrected will most probably solve the whole problem. We will see soon if that is true.

      Update 2019-12-02:
      Trying to fix the leak issue first, it turned out that later during the Python 3.8 development another incompatible change was made. This change modifies the treatment of subtype_dealloc in a way that again creates a reference leak. We found it again with (this time only 10) logarithmic search over PySide builds and tests. It can be seen in

      https://bugs.python.org/issue37879

      bpo-37879: Suppress subtype_dealloc decref when base type is a C heap type (GH-15323, GH-16004) (GH-15966)

      We had to model that situation in order to issue the missing Py_DECREF exactly when it is needed. It still needs verification that this case is correct in every situation.

      Update 2019-12-03:
      The problem is maybe not finally solved, but circumvented in a very compatible way.
      For some reason that is still not clear, PySide cannot stand when PyType_Ready is run
      on a new PySide type and the Py_TPFLAGS_METHOD_DESCRIPTOR flag is set.

      The simple (but slightly hackish) workaround was: During creation of a new PySide type,
      we remove this flag from the type of type.mro and set it again after creation. This solves the issue
      completely.

      We will continue to investigate how to remove this problem completely, but for now
      this intermediate solution is enough to claim that Python 3.8 is supported.

      Summary:
      In effect, this 3.8 version had three incompatible changes every three months. This is quite uncommon for Python, to say the least. We are happy to have a working version now, after allĀ 

      This version is now completely supporting the Limited API (no dependency of the Python version).

        Attachments

          Issue Links

          For Gerrit Dashboard: PYSIDE-939
          # Subject Branch Project Status CR V

            Activity

              People

              • Assignee:
                ctismer Christian Tismer
                Reporter:
                ctismer Christian Tismer
              • Votes:
                6 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Gerrit Reviews

                  There are no open Gerrit changes