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

Incorrect Enum memory management leads to crash on exit

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Open
    • Priority: P2: Important
    • Resolution: Unresolved
    • Affects Version/s: 5.12.0, dev
    • Fix Version/s: dev
    • Component/s: Shiboken
    • Labels:
    • Environment:
      Ubuntu 15.10
      Qt 5.6.x
      Debug-enabled Python 3.x
    • Commits:
      6c018822ba40cc0a4427bd3f9ab8c96829c207df

      Description

      When building PySide2 with a Python 3 debug interpreter on Linux, all tests crash with a segmentation fault on application exit.

      GDB Backtrace at crash:

      1 visit_decref        gcmodule.c    374  0x7ffff7a213c5 
      2 dict_traverse       dictobject.c  3047 0x7ffff7906b84 
      3 subtract_refs       gcmodule.c    399  0x7ffff7a20a97 
      4 collect             gcmodule.c    956  0x7ffff7a21dac 
      5 _PyGC_CollectNoFail gcmodule.c    1623 0x7ffff7a22784 
      6 PyImport_Cleanup    import.c      473  0x7ffff79f2e95 
      7 Py_FinalizeEx       pylifecycle.c 612  0x7ffff7a00169 
      8 Py_Main             main.c        829  0x7ffff7a209b6 
      9 main                python.c      69   0x400bfb       
      

      Valgrind report at crash:

      valgrind --num-callers=40 --suppressions=/home/alex/Dev/python3_source/Misc/valgrind-python.supp --leak-check=no --track-origins=yes python /home/alex/Dev/pyside-setup/sources/shiboken2/tests/otherbinding/collector_external_operator_test.py
      ==23960== Memcheck, a memory error detector
      ==23960== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
      ==23960== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
      ==23960== Command: python /home/alex/Dev/pyside-setup/sources/shiboken2/tests/otherbinding/collector_external_operator_test.py
      ==23960== 
      ==23960== Invalid read of size 8
      ==23960==    at 0x500E3C1: visit_decref (gcmodule.c:374)
      ==23960==    by 0x4EF3B83: dict_traverse (dictobject.c:3047)
      ==23960==    by 0x500DA96: subtract_refs (gcmodule.c:399)
      ==23960==    by 0x500EDAB: collect (gcmodule.c:956)
      ==23960==    by 0x500F783: _PyGC_CollectNoFail (gcmodule.c:1623)
      ==23960==    by 0x4FDFE94: PyImport_Cleanup (import.c:473)
      ==23960==    by 0x4FED168: Py_FinalizeEx (pylifecycle.c:612)
      ==23960==    by 0x500D9B5: Py_Main (main.c:829)
      ==23960==    by 0x400BFA: main (python.c:69)
      ==23960==  Address 0x74c79c8 is 40 bytes inside a block of size 80 free'd
      ==23960==    at 0x4C2CE2B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
      ==23960==    by 0x4F0EF5B: _PyMem_RawFree (obmalloc.c:101)
      ==23960==    by 0x4F0F695: _PyMem_DebugRawFree (obmalloc.c:1910)
      ==23960==    by 0x4F0F6B9: _PyMem_DebugFree (obmalloc.c:1995)
      ==23960==    by 0x4F100B2: PyObject_Free (obmalloc.c:503)
      ==23960==    by 0x4F1DDBF: object_dealloc (typeobject.c:3526)
      ==23960==    by 0x4F0C859: _Py_Dealloc (object.c:1786)
      ==23960==    by 0x4EF3E5C: free_keys_object (dictobject.c:561)
      ==23960==    by 0x4EF5AA2: dict_dealloc (dictobject.c:2020)
      ==23960==    by 0x4F0C859: _Py_Dealloc (object.c:1786)
      ==23960==    by 0x4F09255: module_clear (moduleobject.c:704)
      ==23960==    by 0x500DFF5: delete_garbage (gcmodule.c:867)
      ==23960==    by 0x500EEB4: collect (gcmodule.c:1019)
      ==23960==    by 0x500F783: _PyGC_CollectNoFail (gcmodule.c:1623)
      ==23960==    by 0x4FDFC57: PyImport_Cleanup (import.c:420)
      ==23960==    by 0x4FED168: Py_FinalizeEx (pylifecycle.c:612)
      ==23960==    by 0x500D9B5: Py_Main (main.c:829)
      ==23960==    by 0x400BFA: main (python.c:69)
      ==23960==  Block was alloc'd at
      ==23960==    at 0x4C2BBCF: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
      ==23960==    by 0x4F0EFD3: _PyMem_RawMalloc (obmalloc.c:73)
      ==23960==    by 0x4F0F58B: _PyMem_DebugRawAlloc (obmalloc.c:1856)
      ==23960==    by 0x4F0F62B: _PyMem_DebugRawMalloc (obmalloc.c:1879)
      ==23960==    by 0x4F0F64C: _PyMem_DebugMalloc (obmalloc.c:1981)
      ==23960==    by 0x4F10029: PyObject_Malloc (obmalloc.c:479)
      ==23960==    by 0x4F0C705: _PyObject_New (object.c:266)
      ==23960==    by 0x81EB0A3: Shiboken::Enum::newItem(_typeobject*, long, char const*) (sbkenum.cpp:526)
      ==23960==    by 0x81EADBD: Shiboken::Enum::createEnumItem(_typeobject*, char const*, long) (sbkenum.cpp:483)
      ==23960==    by 0x81EAE9B: Shiboken::Enum::createGlobalEnumItem(_typeobject*, _object*, char const*, long) (sbkenum.cpp:492)
      ==23960==    by 0x77221A8: PyInit_sample (sample_module_wrapper.cpp:4298)
      ==23960==    by 0x4FE2C8A: _PyImport_LoadDynamicModuleWithSpec (importdl.c:154)
      ==23960==    by 0x4FE08EE: _imp_create_dynamic_impl (import.c:2004)
      ==23960==    by 0x4FE0A0A: _imp_create_dynamic (import.c.h:289)
      ==23960==    by 0x4F08EE4: PyCFunction_Call (methodobject.c:114)
      ==23960==    by 0x4FB4F86: do_call_core (ceval.c:5063)
      ==23960==    by 0x4FC15CF: _PyEval_EvalFrameDefault (ceval.c:3366)
      ==23960==    by 0x4FB5B4D: PyEval_EvalFrameEx (ceval.c:718)
      ==23960==    by 0x4FB666A: _PyEval_EvalCodeWithName (ceval.c:4128)
      ==23960==    by 0x4FB68E0: fast_function (ceval.c:4939)
      ==23960==    by 0x4FB6B6D: call_function (ceval.c:4819)
      ==23960==    by 0x4FC1017: _PyEval_EvalFrameDefault (ceval.c:3284)
      ==23960==    by 0x4FB5B4D: PyEval_EvalFrameEx (ceval.c:718)
      ==23960==    by 0x4FB5C35: _PyFunction_FastCall (ceval.c:4880)
      ==23960==    by 0x4FB684E: fast_function (ceval.c:4915)
      ==23960==    by 0x4FB6B6D: call_function (ceval.c:4819)
      ==23960==    by 0x4FC1017: _PyEval_EvalFrameDefault (ceval.c:3284)
      ==23960==    by 0x4FB5B4D: PyEval_EvalFrameEx (ceval.c:718)
      ==23960==    by 0x4FB5C35: _PyFunction_FastCall (ceval.c:4880)
      ==23960==    by 0x4FB684E: fast_function (ceval.c:4915)
      ==23960==    by 0x4FB6B6D: call_function (ceval.c:4819)
      ==23960==    by 0x4FC1017: _PyEval_EvalFrameDefault (ceval.c:3284)
      ==23960==    by 0x4FB5B4D: PyEval_EvalFrameEx (ceval.c:718)
      ==23960==    by 0x4FB5C35: _PyFunction_FastCall (ceval.c:4880)
      ==23960==    by 0x4FB684E: fast_function (ceval.c:4915)
      ==23960==    by 0x4FB6B6D: call_function (ceval.c:4819)
      ==23960==    by 0x4FC1017: _PyEval_EvalFrameDefault (ceval.c:3284)
      ==23960==    by 0x4FB5B4D: PyEval_EvalFrameEx (ceval.c:718)
      ==23960==    by 0x4FB5C35: _PyFunction_FastCall (ceval.c:4880)
      ==23960==    by 0x4FB684E: fast_function (ceval.c:4915)
      ==23960== 
      ==23960== Invalid read of size 1
      ==23960==    at 0x500E3C5: visit_decref (gcmodule.c:374)
      ==23960==    by 0x4EF3B83: dict_traverse (dictobject.c:3047)
      ==23960==    by 0x500DA96: subtract_refs (gcmodule.c:399)
      ==23960==    by 0x500EDAB: collect (gcmodule.c:956)
      ==23960==    by 0x500F783: _PyGC_CollectNoFail (gcmodule.c:1623)
      ==23960==    by 0x4FDFE94: PyImport_Cleanup (import.c:473)
      ==23960==    by 0x4FED168: Py_FinalizeEx (pylifecycle.c:612)
      ==23960==    by 0x500D9B5: Py_Main (main.c:829)
      ==23960==    by 0x400BFA: main (python.c:69)
      ==23960==  Address 0xdbdbdbdbdbdbdc94 is not stack'd, malloc'd or (recently) free'd
      ==23960== 
      ==23960== 
      ==23960== Process terminating with default action of signal 11 (SIGSEGV)
      ==23960==  General Protection Fault
      ==23960==    at 0x500E3C5: visit_decref (gcmodule.c:374)
      ==23960==    by 0x4EF3B83: dict_traverse (dictobject.c:3047)
      ==23960==    by 0x500DA96: subtract_refs (gcmodule.c:399)
      ==23960==    by 0x500EDAB: collect (gcmodule.c:956)
      ==23960==    by 0x500F783: _PyGC_CollectNoFail (gcmodule.c:1623)
      ==23960==    by 0x4FDFE94: PyImport_Cleanup (import.c:473)
      ==23960==    by 0x4FED168: Py_FinalizeEx (pylifecycle.c:612)
      ==23960==    by 0x500D9B5: Py_Main (main.c:829)
      ==23960==    by 0x400BFA: main (python.c:69)
      ==23960== 
      ==23960== HEAP SUMMARY:
      ==23960==     in use at exit: 2,313,128 bytes in 13,891 blocks
      ==23960==   total heap usage: 47,515 allocs, 33,624 frees, 8,162,153 bytes allocated
      ==23960== 
      ==23960== For a detailed leak analysis, rerun with: --leak-check=full
      ==23960== 
      ==23960== For counts of detected and suppressed errors, rerun with: -v
      ==23960== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
      

      The symptom issue in the attached backtraces is that the Python garbage collector tries to access an Enum object that was already deallocated.

      The core issue is that Enum memory management is implemented incorrectly in Shiboken.

      The architecture of Enum management is as follows:

      • SbkEnumType_Type - The general python enum type which is used to create specific Enum classes (e.g. Qt::KeyboardModifier)
      • SbkEnumType - a specific Enum class created using the above SbkEnumType_Type (e.g Qt::KeyboardModifier)
      • SbkEnumObj - an instance of a specific SbkEnumType, (e.g. Qt::ShiftModifier.)

      Ideally, all of these objects should be memory managed by Python (reference counting, possibly GC support).

      There is a problem though that the refcount of SbkEnumObj is decreased one too many times in sbkenum.cpp createGlobalEnumItem, which causes the crash above. If you remove it, the crash does not happen anymore, but instead you get a memory leak.
      That happens because the owner SbkEnumType instance which is responsible for deleting SbkEnumObj, is deleted too late.

      What's currently implemented is that SbkEnumTypes are simply new'ed and stored in a C++ static vector, which are destroyed after main() finishes, which is too late, because Python is already shutdown at that point.
      So because the memory management of SbkEnumTypes is broken, we get leaks for individual enum objects.

      What should be implemented, is making sure that Python disposes of SbkEnumTypes, but that is not trivial, at least I haven't been able to figure it out completely yet.

      So bandaid fix for now, to transform a crash into a memory leak, is to remove a PyDecref.

      Some small reproducibility notes:
      The crash doesn't happen with a non-debug Python interpreter, but running valgrind on a release interpreter also shows the same uninitialized access, so it's still undefined behavior.

      The crash doesn't happen on a macOS python 3 debug build, nor on Linux python 2 debug build.

        Attachments

          Issue Links

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

            Activity

              People

              • Assignee:
                alexandru.croitor Alexandru Croitor
                Reporter:
                alexandru.croitor Alexandru Croitor
              • Votes:
                1 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:

                  Gerrit Reviews

                  There are no open Gerrit changes