Skip to content

gh-118331: Don't raise an error if tuple allocation fails when clearing weakrefs #118338

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.

Sign up for GitHub

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

Merged
colesbury merged 6 commits into from
Apr 29, 2024
Merged

gh-118331: Don't raise an error if tuple allocation fails when clearing weakrefs #118338

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
195a7e5
Don't raise an error if tuple allocation fails when clearing weakrefs
mpage Apr 26, 2024
99e3f85
Fix test
mpage Apr 26, 2024
632087c
Make test more robust to future changes
mpage Apr 27, 2024
7ba419d
Don't pass object to `PyErr_WriteUnraisable`
mpage Apr 27, 2024
15c85d4
Don't chain exceptions
mpage Apr 29, 2024
728a297
Merge branch 'main' into gh-118331-wr-no-mem
mpage Apr 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Don't raise an error if tuple allocation fails when clearing weakrefs 
It's not safe to raise an exception in `PyObject_ClearWeakRefs()` if one
is not already set, since it may be called by `_Py_Dealloc()` and hit
https://github.com/python/cpython/blob/5a90de0d4cbc151a6deea36a27eb81b192410e56/Objects/object.c#L2843-L2860.

Additionally, make sure we clear the weakrefs even when tuple allocation
fails.

This bug predates gh-111926. If it's getting tickled now, I suspect it's
because we always allocate a tuple when clearing weakrefs that have callbacks.
  • Loading branch information
@mpage
mpage committed Apr 26, 2024
commit 195a7e59cff10be13bc0473bb43517f340d6de35
27 changes: 27 additions & 0 deletions Lib/test/test_weakref.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import threading
import time
import random
import textwrap

from test import support
from test.support import script_helper, ALWAYS_EQ
Expand Down Expand Up @@ -1009,6 +1010,32 @@ def __del__(self): pass
del x
support.gc_collect()

@support.cpython_only
def test_no_memory_when_clearing(self):
# gh-118331: Make sure we do not raise an exception from the destructor
# when clearing weakrefs if allocating the intermediate tuple fails.
code = textwrap.dedent("""
import _testcapi
import weakref

class Obj:
pass

def callback(obj):
pass


obj = Obj()
# The choice of 50 is arbitrary, but must be large enough to ensure
# the allocation won't be serviced by the free list.
wrs = [weakref.ref(obj, callback) for _ in range(50)]
_testcapi.set_nomemory(0)
del obj
""").strip()
res = script_helper.assert_python_failure("-c", code)
stderr = res.err.decode("ascii", "backslashreplace")
self.assertNotRegex(stderr, "_Py_Dealloc: Deallocator of type")


class SubclassableWeakrefTestCase(TestBase):

Expand Down
8 changes: 7 additions & 1 deletion Objects/weakrefobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,13 @@ PyObject_ClearWeakRefs(PyObject *object)
PyObject *exc = PyErr_GetRaisedException();
PyObject *tuple = PyTuple_New(num_weakrefs * 2);
if (tuple == NULL) {
_PyErr_ChainExceptions1(exc);
_PyWeakref_ClearWeakRefsExceptCallbacks(object);
if (exc == NULL) {
PyErr_WriteUnraisable(object);
}
else {
_PyErr_ChainExceptions1(exc);
}
return;
}

Expand Down