Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash on _PyDict_CheckConsistency with an empty instance dict #121860

Closed
pablogsal opened this issue Jul 16, 2024 · 10 comments
Closed

Crash on _PyDict_CheckConsistency with an empty instance dict #121860

pablogsal opened this issue Jul 16, 2024 · 10 comments
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes release-blocker type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@pablogsal
Copy link
Member

pablogsal commented Jul 16, 2024

When running a complicated application that involves Cython I am getting the following crash:

Objects/dictobject.c:716: _PyDict_CheckConsistency: Assertion failed: mp->ma_values->valid == 1
Enable tracemalloc to get the memory block allocation traceback

object address  : 0x21bbd30
object refcount : 1
object type     : 0x9cb9e0
object type name: dict
object repr     : {}

Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: finalizing (tstate=0x0000000000a522b8)

Current thread 0x00007ffff7fe2740 (most recent call first):
  Garbage-collecting
  <no Python frame>

Extension modules: xxx._ext, xxx._ext, xxx._tcp_impl (total: 3)

Thread 1 "python" received signal SIGABRT, Aborted.

I am trying to get a smaller reproducer that doesn't involve the entire world out of the application, but it may take me a while.

Linked PRs

@pablogsal pablogsal added the 3.13 bugs and security fixes label Jul 16, 2024
@Eclips4 Eclips4 added the type-crash A hard crash of the interpreter, possibly with a core dump label Jul 16, 2024
@vstinner
Copy link
Member

vstinner commented Jul 16, 2024

Bug also seen in the Cython test suite: #121253

@pablogsal
Copy link
Member Author

pablogsal commented Jul 16, 2024

This also happens when I did compile
with ./configure --with-pydebug and I'm running with PYTHONDEVMODE=1 PYTHONMALLOC=malloc

@vstinner
Copy link
Member

See also #121863 "CMultiplyNested closure crash: Python 3.13 regression". I don't know if it's related or not, but it was also found with the Cython test suite.

@colesbury
Copy link
Contributor

Repro:

class Foo:
    pass

class StrSubclass(str):
    pass

foo = Foo()
foo.attr = 2
foo.__dict__ = {}
del foo.__dict__

getattr(foo, StrSubclass("attr"))
Objects/dictobject.c:716: _PyDict_CheckConsistency: Assertion failed: mp->ma_values->valid == 1
Enable tracemalloc to get the memory block allocation traceback

object address  : 0x7f4aa716e210
object refcount : 1
object type     : 0x55dc45c3fc20
object type name: dict
object repr     : {}

Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized

Current thread 0x00007f4aa7695740 (most recent call first):
  File "/raid/sgross/cpython/repro.py", line 14 in <module>
Aborted (core dumped)

@colesbury
Copy link
Contributor

_PyObject_MaterializeManagedDict_LockHeld is called to materialize the dictionary:

  • The object doesn't have a materialized dictionary, (otherwise, we return it in _PyObject_MaterializeManagedDict)
  • But the inline values may still not be valid! For example, if we materialize a dictionary and then delete it, the inline values are not valid, but there is no dictionary.

@colesbury colesbury added the 3.14 new features, bugs and security fixes label Jul 16, 2024
@vstinner
Copy link
Member

Bug also seen in the Cython test suite: #121253

I can reliably reproduce the error on Linux by running the Cython test dealloc_raise:

$ ~/python/3.13/python runtests.py -vv dealloc_raise
(...)
'/home/vstinner/python/3.13/python test_dealloc_raise1.py'
-- stdout:
-- stderr:Objects/dictobject.c:716: _PyDict_CheckConsistency: Assertion failed: mp->ma_values->valid == 1
Enable tracemalloc to get the memory block allocation traceback

object address  : 0x7fc6a6b7a3f0
object refcount : 1
object type     : 0xa5a100
object type name: dict
object repr     : {}

Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized

Current thread 0x00007fc6b48e3740 (most recent call first):
  File "/home/vstinner/dev/cython/TEST_TMP/run/dealloc_raise/test_dealloc_raise1.py", line 10 in <module>
(...)

colesbury added a commit to colesbury/cpython that referenced this issue Jul 16, 2024
The object's inline values may be marked invalid if the materialized
dict was already initialized and then deleted.
@colesbury
Copy link
Contributor

@vstinner, can you check if this patch fixes the Cython crash as well? #121866

@pablogsal
Copy link
Member Author

pablogsal commented Jul 16, 2024

Bisected to:

8b541c017ea92040add608b3e0ef8dc85e9e6060 is the first bad commit
commit 8b541c017ea92040add608b3e0ef8dc85e9e6060
Author: Dino Viehland <dinoviehland@meta.com>
Date:   Sun Apr 21 22:57:05 2024 -0700

    gh-112075: Make instance attributes stored in inline "dict" thread safe (#114742)

    Make instance attributes stored in inline "dict" thread safe on free-threaded builds

 Include/cpython/object.h                       |   1 +
 Include/internal/pycore_dict.h                 |  19 +-
 Include/internal/pycore_object.h               |  16 +-
 Include/internal/pycore_pyatomic_ft_wrappers.h |  14 +
 Lib/test/test_class.py                         |   9 +
 Objects/dictobject.c                           | 385 ++++++++++++++++++++-----
 Objects/object.c                               |  45 ++-
 Objects/typeobject.c                           |  30 +-
 Python/bytecodes.c                             |  15 +-
 Python/executor_cases.c.h                      |  10 +-
 Python/generated_cases.c.h                     |  13 +-
 Python/specialize.c                            |   3 +-
 Tools/cases_generator/analyzer.py              |   1 +
 13 files changed, 419 insertions(+), 142 deletions(-)
bisect found first bad commit

@pablogsal
Copy link
Member Author

pablogsal commented Jul 16, 2024

@vstinner, can you check if this patch fixes the Cython crash as well? #121866

@colesbury I can confirm it works:

Before

../python/3.13/python.exe  runtests.py -vv dealloc_raise
Python 3.13.0b3+ (heads/3.13:cd74ed0a71f, Jul 16 2024, 19:24:17) [Clang 15.0.0 (clang-1500.3.9.4)]

Running tests against Cython 3.1.0a0 746627fdca5e48db2b63c8bb4be82b33ea617549
Using Cython language level 2.
Test dependency not found: 'numpy'
Test dependency not found: 'pythran'
/Users/pgalindo3/.local/lib/python3.13/site-packages/setuptools/sandbox.py:13: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  import pkg_resources
Test dependency found: 'setuptools.sandbox' version 69.1.1
Test dependency found: 'asyncio' version 3.13.0b3+
Test dependency found: 'pstats' version 3.13.0b3+
Test dependency found: 'posix' version 3.13.0b3+
Test dependency found: 'array' version ?.?
Test dependency found: 'Cython.Coverage' version 3.1.0a0
Test dependency found: 'Cython.Coverage' version 3.1.0a0
Test dependency not found: 'IPython.testing.globalipapp'
Test dependency not found: 'jedi_BROKEN_AND_DISABLED'
Test dependency found: 'test.support' version 3.13.0b3+
Backends: c,cpp

runTest (__main__.EndToEndTest.runTest)
[-1] End-to-end dealloc_raise ... [-1] ['/Users/pgalindo3/github/cython/../python/3.13/python.exe', 'setup.py', 'build_ext', '--inplace']
Compiling dealloc_raise.pyx because it changed.
[1/1] Cythonizing dealloc_raise.pyx

/Users/pgalindo3/github/cython/Cython/Compiler/Main.py:373: FutureWarning: Cython directive 'language_level' not set, using '3' (Py3). This has changed from earlier releases! File: /Users/pgalindo3/github/cython/TEST_TMP/run/dealloc_raise/dealloc_raise.pyx
  tree = Parsing.p_module(s, pxd, full_module_name)


[-1] ['/Users/pgalindo3/github/cython/../python/3.13/python.exe', 'test_dealloc_raise1.py']

Objects/dictobject.c:716: _PyDict_CheckConsistency: Assertion failed: mp->ma_values->valid == 1
Enable tracemalloc to get the memory block allocation traceback

object address  : 0x102c1ccb0
object refcount : 1
object type     : 0x1026e5f50
object type name: dict
object repr     : {}

Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized

Current thread 0x0000000208024c00 (most recent call first):
  File "/Users/pgalindo3/github/cython/TEST_TMP/run/dealloc_raise/test_dealloc_raise1.py", line 10 in <module>

Extension modules: dealloc_raise (total: 1)

Final directory layout of 'dealloc_raise':
./test_dealloc_raise1.py
./dealloc_raise.pyx
./dealloc_raise.c
./dealloc_raise.cpython-313d-darwin.so
./setup.py
./test_dealloc_raise2.py
./build/temp.macosx-14.5-arm64-cpython-313-pydebug/dealloc_raise.o
./build/lib.macosx-14.5-arm64-cpython-313-pydebug/dealloc_raise.cpython-313d-darwin.so

FAIL

======================================================================
FAIL: runTest (__main__.EndToEndTest.runTest)
[-1] End-to-end dealloc_raise
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/pgalindo3/github/cython/runtests.py", line 2040, in runTest
    self.assertEqual(0, res, "non-zero exit status, last output was:\n%r\n-- stdout:%s\n-- stderr:%s\n" % (
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        ' '.join(command), out[-1], err[-1]))
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 0 != -6 : non-zero exit status, last output was:
'/Users/pgalindo3/github/cython/../python/3.13/python.exe test_dealloc_raise1.py'
-- stdout:
-- stderr:Objects/dictobject.c:716: _PyDict_CheckConsistency: Assertion failed: mp->ma_values->valid == 1
Enable tracemalloc to get the memory block allocation traceback

object address  : 0x102c1ccb0
object refcount : 1
object type     : 0x1026e5f50
object type name: dict
object repr     : {}

Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized

Current thread 0x0000000208024c00 (most recent call first):
  File "/Users/pgalindo3/github/cython/TEST_TMP/run/dealloc_raise/test_dealloc_raise1.py", line 10 in <module>

Extension modules: dealloc_raise (total: 1)



----------------------------------------------------------------------
Ran 1 test in 1.905s

FAILED (failures=1)
Most expensive pipeline stages:
Times:
etoe-build  :     1.18 sec  (   1,  1.177 / run) - slowest: 'c:dealloc_raise(1)' (1.18s)
etoe-run    :     0.25 sec  (   1,  0.253 / run) - slowest: 'c:dealloc_raise(2)' (0.25s)
ALL DONE

After

../python/3.13/python.exe  runtests.py -vv dealloc_raise
^[[APython 3.13.0b3+ (heads/3.13-dirty:cd74ed0a71f, Jul 16 2024, 19:21:10) [Clang 15.0.0 (clang-1500.3.9.4)]

Running tests against Cython 3.1.0a0 746627fdca5e48db2b63c8bb4be82b33ea617549
Using Cython language level 2.
Test dependency not found: 'numpy'
Test dependency not found: 'pythran'
/Users/pgalindo3/.local/lib/python3.13/site-packages/setuptools/sandbox.py:13: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  import pkg_resources
Test dependency found: 'setuptools.sandbox' version 69.1.1
Test dependency found: 'asyncio' version 3.13.0b3+
Test dependency found: 'pstats' version 3.13.0b3+
Test dependency found: 'posix' version 3.13.0b3+
Test dependency found: 'array' version ?.?
Test dependency found: 'Cython.Coverage' version 3.1.0a0
Test dependency found: 'Cython.Coverage' version 3.1.0a0
Test dependency not found: 'IPython.testing.globalipapp'
Test dependency not found: 'jedi_BROKEN_AND_DISABLED'
Test dependency found: 'test.support' version 3.13.0b3+
Backends: c,cpp

runTest (__main__.EndToEndTest.runTest)
[-1] End-to-end dealloc_raise ... ok

----------------------------------------------------------------------
Ran 1 test in 3.411s

OK
Most expensive pipeline stages: 'AnalyseDeclarationsTransform': 0.42 / 8 (0.053 / run, 53.7%), 'parse': 0.11 / 8 (0.014 / run, 14.4%), 'generate_pyx_code_stage': 0.07 / 1 (0.069 / run, 8.8%), 'AnalyseExpressionsTransform': 0.02 / 8 (0.002 / run, 1.9%), 'RemoveUnreachableCode': 0.01 / 16 (0.001 / run, 1.7%), 'OptimizeBuiltinCalls': 0.01 / 8 (0.002 / run, 1.6%), 'PostParse': 0.01 / 8 (0.001 / run, 1.2%), 'MarkClosureVisitor': 0.01 / 8 (0.001 / run, 1.2%), 'InterpretCompilerDirectives': 0.01 / 8 (0.001 / run, 1.2%), 'ControlFlowAnalysis': 0.01 / 8 (0.001 / run, 1.0%)
Times:
etoe-build  :     2.33 sec  (   1,  2.331 / run) - slowest: 'c:dealloc_raise(1)' (2.33s)
etoe-run    :     1.07 sec  (   2,  0.534 / run) - slowest: 'c:dealloc_raise(2)' (0.70s), 'c:dealloc_raise(3)' (0.37s)
ALL DONE

colesbury added a commit that referenced this issue Jul 16, 2024
The object's inline values may be marked invalid if the materialized
dict was already initialized and then deleted.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 16, 2024
…121866)

The object's inline values may be marked invalid if the materialized
dict was already initialized and then deleted.
(cherry picked from commit 162b41f)

Co-authored-by: Sam Gross <colesbury@gmail.com>
colesbury added a commit that referenced this issue Jul 16, 2024
… (#121867)

The object's inline values may be marked invalid if the materialized
dict was already initialized and then deleted.
(cherry picked from commit 162b41f)

Co-authored-by: Sam Gross <colesbury@gmail.com>
@colesbury
Copy link
Contributor

I think this is fixed now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes release-blocker type-crash A hard crash of the interpreter, possibly with a core dump
Projects
Development

No branches or pull requests

4 participants