Calling Py_DECREF twice does not result in error with debug build

[kevinAlbs]

Bug report

Bug description:

Documented behavior of Py_REF_DEBUG includes:

Py_REF_DEBUG also checks after every decref to verify that the refcount hasn't
gone negative, and causes an immediate fatal error if it has.

Calling Py_DECREF twice does not result in the expected error:

static PyObject* spam_double_decref(PyObject *self, PyObject *args) {
    printf("spam_double_decref ... begin\n");
    PyObject *obj = Py_BuildValue("s", "foobar");
    Py_DECREF (obj);
    Py_DECREF (obj); // Expect error, but does not error when using cpython built with `--with-pydebug`.
    printf("spam_double_decref ... end\n");
    Py_RETURN_NONE;
}

To reproduce:

Build CPython with --with-pydebug:

cd /home/kevin/code/cpython
mkdir debug
cd debug/
../configure --with-pydebug
make -j16

Build this sample extension calling Py_DECREF twice using the debug build of CPython:

PYTHON=/home/kevin/code/cpython/debug/python
$PYTHON setup.py build

Run a test with this extension:

# Add path to built extension to `PYTHONPATH`.
export PYTHONPATH=/home/kevin/code/cpython/KEVINALBS/double_decref_extension/build/lib.linux-x86_64-cpython-313-pydebug
PYTHON=/home/kevin/code/cpython/debug/python
$PYTHON -c "import spam; spam.double_decref()"
# Prints:
# spam_double_decref ... begin
# spam_double_decref ... end

No error is indicated, but an error is expected.

Extension source is located here: https://github.com/kevinAlbs/double_decref_extension
Tested with cpython main branch on commit: 929cc4e.

If this issue is confirmed, I may be interested to investigate possible solutions.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

• gh-109496: Detect Py_DECREF() after dealloc in debug mode #109539
• [3.12] gh-109496: Detect Py_DECREF() after dealloc in debug mode (GH-109539) #109545
• gh-109496: Skip test_capi.test_decref_freed_object() on ASAN #109573
• [3.12] gh-109496: Skip test_capi.test_decref_freed_object() on ASAN (GH-109573) #109578

[terryjreedy]

What happens if you print the refcount before and after each decref?

[sorcio]

I think that snippet about Py_REF_DEBUG might have misled you. It doesn't do what you think it does.

What happens is that the first Py_DECREF correctly decreases the ref count to 0, and deallocates the object. Now obj is a dangling pointer. The second Py_DECREF will receive this dangling pointer, and what happens at this point is undefined behavior. It's a property of user code that an object reference is never used after it's released, and it's up to the developer to guarantee it.

---

There might be an interesting bit here thanks to the new immortality check. There is a chance that, on debug builds, dereferencing garbage might look like an object with the immortality bit set, and Py_DECREF will just ignore it. Before the immortality check was there, it would probably have crashed or reported the error in one way or another, I guess?

[terryjreedy]

decref subtracts 1 and only ends at 0 if the refcount really started at 1. If it somehow was 2, 2 decrefs are not a bug. I know this seems impossible but so seems the observed behavior. Hence the debug suggestion to confirm the presumption.

[serhiy-storchaka]

It is an interesting example in which the debug build hides a bug which can be exposed in the release build. In the debug build the deallocated memory if filled with some recognizable pattern. Therefore, bytes which contained the refcount no longer zero, and the second Py_DECREF() simply change these bydes, but since they are not zero and not 1, it doesn't do anything more. This is a reminder that we should not only test on the debug build, but also on the release build.

@vstinner, it may be interesting to you.

It is not Python fault. Just does not do this. You should only call Py_DECREF() if you own a reference, and you should only call Py_DECREF() twice if you own it twice. Python cannot help here. Even if it can detect that the object has been deallocated, it won't be able to determine what Py_DECREF() was wrong -- the last call or the one that was called an hour ago in other extension code.

[vstinner]

I wrote PR #109539 to fix this Python 3.12 regression caused by immortal objects (PEP 683).

---

When an object reference count if 1, Py_DECREF() calls _Py_Dealloc(). For example, for a Python str object, _Py_Dealloc() calls PyObject_Free(). Using PYTHONMALLOC=debug or a debug build of Python, PyObject_Free() calls memset(q, PYMEM_DEADBYTE, nbytes) before calling _PyObject_Free() which deallocates memory.

At the end, the first Py_DECREF(obj) creates a dangling pointer: using obj is now undefined. But well, in most cases, dereferencing the pointer will not crash since Python rarely really gives the memory back to the system (kernel), but keeps it for performance. And so no SIGSEGV or SIGBUS is sent to Python. For example, pymalloc (used for memory blocks up to 512 bytes) only gives an arena of 256 KiB back to the system (kernel) once all* memory of this arena is freed... and memory fragmentation makes this event "rare".

In short, the first Py_DECREF() sets PyObject.ob_refcnt to 0xDDDD...DDDD. And now the magic happens. PEP 683 added the following code to Py_DECREF():

    if (_Py_IsImmortal(op)) {
        return;
    }

And and and... this reference count is considered as immortal on x86-64 (64-bit void* pointer)! Example in gdb after the object is deallocated by the first Py_DECREF():

(gdb) p obj->ob_refcnt
$1 = -2459565876494606883
(gdb) p /x obj->ob_refcnt
$2 = 0xdddddddddddddddd
(gdb) p _Py_IsImmortal(obj)
$3 = 1

There are different options to trigger an error on the second Py_DECREF():

• Check for _PyObject_IsFreed() in Py_DECREF() -- it can slow down Python debug build, and requires to move _PyObject_IsFreed() from the internal C API to the public C API :-(
• Hack the memory allocator to set PyObject.ob_refcnt to 0, -1 or +1 so the next Py_DECREF() call will call _Py_NegativeRefcount().
• Change Py_DECREF() to check for (op->ob_refcnt < 0) earlier and call _Py_NegativeRefcount() in this case -- my favorite option
• Change _Py_IsImmortal() to not consider 0xdddddddddddddddd reference count as immortal. -- change the logic, or use _PyMem_IsPtrFreed(), which implies to move _PyMem_IsPtrFreed() definition from internal API to the public C API (when Py_REF_DEBUG is defined).

My PR checks Py_DECREF() to always check (op->ob_refcnt < 0) and do the check before checking _Py_IsImmortal().

[kevinAlbs]

What happens if you print the refcount before and after each decref?

spam_double_decref ... begin
before first Py_DECREF  : obj->ob_refcnt=1 (0x1)
after first Py_DECREF   : obj->ob_refcnt=-2459565876494606883 (0xdddddddddddddddd)
after second Py_DECREF  : obj->ob_refcnt=-2459565876494606883 (0xdddddddddddddddd)
spam_double_decref ... end

@vstinner thank you for the interesting explanation and quick fix! Confirmed using commit 0bb0d88 running the original example produces an expected error:

spam_double_decref ... begin
before first Py_DECREF  : obj->ob_refcnt=1 (0x1)
after first Py_DECREF   : obj->ob_refcnt=-2459565876494606883 (0xdddddddddddddddd)
spammodule.c:10: _Py_NegativeRefcount: Assertion failed: object has negative ref count
<object at 0x7f55679d6b60 is freed>
Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized

Current thread 0x00007f5568be2740 (most recent call first):
  File "<string>", line 1 in <module>

Extension modules: spam (total: 1)

Closing issue.

[vstinner]

@vstinner thank you for the interesting explanation and quick fix!

You're welcome.

Confirmed using commit 0bb0d88 running the original example produces an expected error

Good that you validated manualy the fix! I wrote a test to check for non-regression.

[vstinner]

Ah, and thanks for your bug report! Apparently, nobody noticed the issue before!