10000 gh-77894: Fix a crash when the GC breaks a loop containing a memoryvi… · python/cpython@a1dbf2e · GitHub
[go: up one dir, main page]
Skip to content

Commit a1dbf2e

Browse files
serhiy-storchakaserhiy-storchaka
authored
gh-77894: Fix a crash when the GC breaks a loop containing a memoryview (GH-123898)
Now a memoryview object can only be cleared if there are no buffers that refer it.
1 parent 00ffdf2 commit a1dbf2e

File tree

4 files changed

+57
-33
lines changed

4 files changed

+57
-33
lines changed

‎Lib/test/pickletester.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2323,12 +2323,10 @@ def test_picklebuffer_error(self):
23232323
'PickleBuffer can only be pickled with protocol >= 5')
23242324

23252325
def test_non_continuous_buffer(self):
2326-
if self.pickler is pickle._Pickler:
2327-
self.skipTest('CRASHES (see gh-122306)')
23282326
for proto in protocols[5:]:
23292327
with self.subTest(proto=proto):
23302328
pb = pickle.PickleBuffer(memoryview(b"foobar")[::2])
2331-
with self.assertRaises(pickle.PicklingError):
2329+
with self.assertRaises((pickle.PicklingError, BufferError)):
23322330
self.dumps(pb, proto)
23332331

23342332
def test_buffer_callback_error(self):

‎Lib/test/test_memoryview.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@
1818
from test.support import import_helper
1919

2020

21+
class MyObject:
22+
pass
23+
24+
2125
class AbstractMemoryTests:
2226
source_bytes = b"abcdef"
2327

@@ -228,8 +232,6 @@ def __init__(self, base):
228232
self.m = memoryview(base)
229233
class MySource(tp):
230234
pass
231-
class MyObject:
232-
pass
233235

234236
# Create a reference cycle through a memoryview object.
235237
# This exercises mbuf_clear().
@@ -656,5 +658,26 @@ def __bool__(self):
656658
m[0] = MyBool()
657659
self.assertEqual(ba[:8], b'\0'*8)
658660

661+
def test_buffer_reference_loop(self):
662+
m = memoryview(b'abc').__buffer__(0)
663+
o = MyObject()
664+
o.m = m
665+
o.o = o
666+
wr = weakref.ref(o)
667+
del m, o
668+
gc.collect()
669+
self.assertIsNone(wr())
670+
671+
def test_picklebuffer_reference_loop(self):
672+
pb = pickle.PickleBuffer(memoryview(b'abc'))
673+
o = MyObject()
674+
o.pb = pb
675+
o.o = o
676+
wr = weakref.ref(o)
677+
del pb, o
678+
gc.collect()
679+
self.assertIsNone(wr())
680+
681+
659682
if __name__ == "__main__":
660683
unittest.main()

‎Misc/NEWS.d/next/Core_and_Builtins/2024-09-10-13-27-16.gh-issue-77894.ZC-Olu.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix possible crash in the garbage collector when it tries to break a
2+
reference loop containing a :class:`memoryview` object. Now a
3+
:class:`!memoryview` object can only be cleared if there are no buffers that
4+
refer it.

‎Objects/memoryobject.c

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,6 @@ mbuf_release(_PyManagedBufferObject *self)
109109
if (self->flags&_Py_MANAGED_BUFFER_RELEASED)
110110
return;
111111

112-
/* NOTE: at this point self->exports can still be > 0 if this function
113-
is called from mbuf_clear() to break up a reference cycle. */
114112
self->flags |= _Py_MANAGED_BUFFER_RELEASED;
115113

116114
/* PyBuffer_Release() decrements master->obj and sets it to NULL. */
@@ -1096,32 +1094,19 @@ PyBuffer_ToContiguous(void *buf, const Py_buffer *src, Py_ssize_t len, char orde
10961094
/* Inform the managed buffer that this particular memoryview will not access
10971095
the underlying buffer again. If no other memoryviews are registered with
10981096
the managed buffer, the underlying buffer is released instantly and
1099-
marked as inaccessible for both the memoryview and the managed buffer.
1100-
1101-
This function fails if the memoryview itself has exported buffers. */
1102-
static int
1097+
marked as inaccessible for both the memoryview and the managed buffer. */
1098+
static void
11031099
_memory_release(PyMemoryViewObject *self)
11041100
{
1101+
assert(self->exports == 0);
11051102
if (self->flags & _Py_MEMORYVIEW_RELEASED)
1106-
return 0;
1103+
return;
11071104

1108-
if (self->exports == 0) {
1109-
self->flags |= _Py_MEMORYVIEW_RELEASED;
1110-
assert(self->mbuf->exports > 0);
1111-
if (--self->mbuf->exports == 0)
1112-
mbuf_release(self->mbuf);
1113-
return 0;
1105+
self->flags |= _Py_MEMORYVIEW_RELEASED;
1106+
assert(self->mbuf->exports > 0);
1107+
if (--self->mbuf->exports == 0) {
1108+
mbuf_release(self->mbuf);
11141109
}
1115-
if (self->exports > 0) {
1116-
PyErr_Format(PyExc_BufferError,
1117-
"memoryview has %zd exported buffer%s", self->exports,
1118-
self->exports==1 ? "" : "s");
1119-
return -1;
1120-
}
1121-
1122-
PyErr_SetString(PyExc_SystemError,
1123-
"_memory_release(): negative export count");
1124-
return -1;
11251110
}
11261111

11271112
/*[clinic input]
@@ -1134,9 +1119,21 @@ static PyObject *
11341119
memoryview_release_impl(PyMemoryViewObject *self)
11351120
/*[clinic end generated code: output=d0b7e3ba95b7fcb9 input=bc71d1d51f4a52f0]*/
11361121
{
1137-
if (_memory_release(self) < 0)
1122+
if (self->exports == 0) {
1123+
_memory_release(self);
1124+
Py_RETURN_NONE;
1125+
}
1126+
1127+
if (self->exports > 0) {
1128+
PyErr_Format(PyExc_BufferError,
1129+
"memoryview has %zd exported buffer%s", self->exports,
1130+
self->exports==1 ? "" : "s");
11381131
return NULL;
1139-
Py_RETURN_NONE;
1132+
}
1133+
1134+
PyErr_SetString(PyExc_SystemError,
1135+
"memoryview: negative export count");
1136+
return NULL;
11401137
}
11411138

11421139
static void
@@ -1145,7 +1142,7 @@ memory_dealloc(PyObject *_self)
11451142
PyMemoryViewObject *self = (PyMemoryViewObject *)_self;
11461143
assert(self->exports == 0);
11471144
_PyObject_GC_UNTRACK(self);
1148-
(void)_memory_release(self);
1145+
_memory_release(self);
11491146
Py_CLEAR(self->mbuf);
11501147
if (self->weakreflist != NULL)
11511148
PyObject_ClearWeakRefs((PyObject *) self);
@@ -1164,8 +1161,10 @@ static int
11641161
memory_clear(PyObject *_self)
11651162
{
11661163
PyMemoryViewObject *self = (PyMemoryViewObject *)_self;
1167-
(void)_memory_release(self);
1168-
Py_CLEAR(self->mbuf);
1164+
if (self->exports == 0) {
1165+
_memory_release(self);
1166+
Py_CLEAR(self->mbuf);
1167+
}
11691168
return 0;
11701169
}
11711170

0 commit comments

Comments
 (0)
0