`deque_clear` clears `MemoryError`

[brijkapadia]

Bug report

Bug description:

This is a sub-issue for gh-146102 with the gist here.

In the case that there is no memory, deque_clear calls PyErr_Clear():

cpython/Modules/_collectionsmodule.c

Lines 749 to 753 in 7928a8b

	b = newblock(deque);
	if (b == NULL) {
	PyErr_Clear();
	goto alternate_method;
	}

This was done to clear the error that newblock raises. However, if there was originally an exception, deque_clear clears it.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

• gh-152017: Fix deque_clear to no longer alters exceptions #152018

[rhettinger]

What is wrong with the current code? If an allocation fails due to lack of memory (a very rare almost non-existent event), then an alternate path is taken that does not require additional memory.

AFAICT this is a false positive from the umbrella issue.

[brijkapadia]

AFAICT this is a false positive from the umbrella issue.

There is a real error here that the reproducer catches. However, the umbrella issue gives the wrong explanation. The problem has nothing to do with deque.copy but deque.copy is one execution path for the error.

Reproducer (for reference)

import _testcapi
from collections import deque

for n in range(1, 80):
    d = deque(range(200))
    _testcapi.set_nomemory(n, 0)
    try:
        d2 = d.copy()
        _testcapi.remove_mem_hooks()
        break
    except MemoryError:
        _testcapi.remove_mem_hooks()
    except:
        _testcapi.remove_mem_hooks()
# Fatal: _Py_Dealloc: Deallocator of type 'collections.deque' cleared

The real problem is that this debug check in _Py_Dealloc fails:

Debug Check

cpython/Objects/object.c

Lines 3321 to 3342 in 1812162

	#ifdef Py_DEBUG
	// gh-89373: The tp_dealloc function must leave the current exception
	// unchanged.
	if (tstate != NULL && tstate->current_exception != old_exc) {
	const char *err;
	if (old_exc == NULL) {
	err = "Deallocator of type '%s' raised an exception";
	}
	else if (tstate->current_exception == NULL) {
	err = "Deallocator of type '%s' cleared the current exception";
	}
	else {
	// It can happen if dealloc() normalized the current exception.
	// A deallocator function must not change the current exception,
	// not even normalize it.
	err = "Deallocator of type '%s' overrode the current exception";
	}
	_Py_FatalErrorFormat(__func__, err, type->tp_name);
	}
	Py_XDECREF(old_exc);
	Py_DECREF(type);
	#endif

---

For the specific case of the reproducer, deque_copy_impl calls deque_append_lock_held and in the case of an error, calls Py_DECREF(new_deque):

cpython/Modules/_collectionsmodule.c

Lines 634 to 638 in 1812162

	if (deque_append_lock_held(new_deque, Py_NewRef(item),
	new_deque->maxlen) < 0) {
	Py_DECREF(new_deque);
	return NULL;
	}

Furthermore, deque_append_lock_held calls newblock which raises a PyErr_NoMemory() in the case of OOM:

cpython/Modules/_collectionsmodule.c

Line 187 in 1812162

PyErr_NoMemory();

Thus, in the case of OOM, Py_DECREF(new_deque) is called with an exception already set.

In order to free the memory, eventually deque_clear is called. deque_clear calls newblock, which in the case of OOM, replaces the current exception with a new PyErr_NoMemory(). Then, deque_clear calls PyErr_Clear(), meaning that there are now no exceptions despite deque_append_lock_held originally setting one:

cpython/Modules/_collectionsmodule.c

Lines 749 to 753 in 1812162

	b = newblock(deque);
	if (b == NULL) {
	PyErr_Clear();
	goto alternate_method;
	}

In debug builds, this is caught in _Py_Dealloc,

cpython/Objects/object.c

Lines 3329 to 3331 in 1812162

	else if (tstate->current_exception == NULL) {
	err = "Deallocator of type '%s' cleared the current exception";
	}

which gives the error Fatal: _Py_Dealloc: Deallocator of type 'collections.deque' cleared the current exception.