Skip to content

Commit 06846a9

Browse files
committed
gh-142663: resolving comments by refactoring unit tests and cache type check result
1 parent edd566c commit 06846a9

3 files changed

Lines changed: 27 additions & 62 deletions

File tree

Lib/test/test_memoryview.py

Lines changed: 21 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -228,66 +228,31 @@ def test_compare(self):
228228
self.assertRaises(TypeError, lambda: m >= c)
229229
self.assertRaises(TypeError, lambda: c > m)
230230

231-
def test_compare_use_after_free(self):
232-
# Prevent crash in comparisons of memoryview objs with re-entrant struct.unpack_from.
231+
def test_compare_1d_concurrent_mutation(self):
232+
# Prevent crashes during a mixed format 1-D comparison loop.
233233
# Regression test for https://github.com/python/cpython/issues/142663.
234+
src1 = array.array("d", [1.0, 2.0])
235+
src2 = array.array("l", [1, 2])
236+
mv1, mv2 = memoryview(src1), memoryview(src2)
237+
self.do_test_compare_concurrent_mutation(src1, mv1, mv2)
234238

235-
class ST(struct.Struct):
236-
# Context set by the subtests
237-
view = None
238-
source = None
239-
239+
def test_compare_2d_concurrent_mutation(self):
240+
# Prevent crashes during a mixed format 2-D comparison loop.
241+
# Regression test for https://github.com/python/cpython/issues/142663.
242+
src1 = array.array("d", [1.0, 2.0])
243+
src2 = array.array("l", [1, 2])
244+
mv1 = memoryview(src1).cast("B").cast("d", shape=(1, 2))
245+
mv2 = memoryview(src2).cast("B").cast("l", shape=(1, 2))
246+
self.do_test_compare_concurrent_mutation(src1, mv1, mv2)
247+
248+
def do_test_compare_concurrent_mutation(self, src1, mv1, mv2):
249+
class S(struct.Struct):
240250
def unpack_from(self, buf, /, offset=0):
241-
# Attempt to release the buffer while it's being used in comparison loop.
242-
if self.view is not None:
243-
self.view.release()
244-
245-
# array resize invalidates the buffer pointer used by the comparison loop.
246-
if self.source is not None:
247-
self.source.append(3.14)
248-
251+
mv1.release()
252+
src1.append(3.14)
249253
return (1,)
250-
251-
with support.swap_attr(struct, "Struct", ST):
252-
# Case 1: 1-D comparison (uses cmp_base optimized loop)
253-
# Use mixed types ('d' vs 'l') to force struct unpacking path.
254-
with self.subTest(ndim=1):
255-
a = array.array("d", [1.0, 2.0])
256-
b = array.array("l", [1, 2])
257-
mv_a = memoryview(a)
258-
mv_b = memoryview(b)
259-
260-
ST.view = mv_a
261-
ST.source = a
262-
try:
263-
with self.assertRaises(BufferError):
264-
# Expect BufferError because the memoryview is locked during comparison
265-
mv_a == mv_b
266-
finally:
267-
ST.view = None
268-
ST.source = None
269-
mv_a.release()
270-
mv_b.release()
271-
272-
# Case 2: N-D comparison (uses cmp_rec recursive function)
273-
# Use mixed types ('d' vs 'l') to force struct unpacking path.
274-
with self.subTest(ndim=2):
275-
a = array.array("d", [1.0, 2.0])
276-
b = array.array("l", [1, 2])
277-
mv_a = memoryview(a).cast("B").cast("d", shape=(1, 2))
278-
mv_b = memoryview(b).cast("B").cast("l", shape=(1, 2))
279-
280-
ST.view = mv_a
281-
ST.source = a
282-
try:
283-
with self.assertRaises(BufferError):
284-
# Expect BufferError because the memoryview is locked during comparison
285-
mv_a == mv_b
286-
finally:
287-
ST.view = None
288-
ST.source = None
289-
mv_a.release()
290-
mv_b.release()
254+
with support.swap_attr(struct, "Struct", S):
255+
self.assertRaises(BufferError, mv1.__eq__, mv2)
291256

292257
def check_attributes_with_type(self, tp):
293258
m = self._view(tp(self._source))
Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,2 @@
1-
:class:`memoryview`: Fix a use-after-free crash during comparison when an
2-
overridden :meth:`struct.Struct.unpack_from` releases and resizes the
3-
underlying buffer.
1+
Fix use-after-free crashes when a :class:`memoryview` is mutated
2+
during a comparison with another object.

Objects/memoryobject.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3122,7 +3122,8 @@ memory_richcompare(PyObject *v, PyObject *w, int op)
31223122
}
31233123
vv = VIEW_ADDR(v);
31243124

3125-
if (PyMemoryView_Check(w)) {
3125+
int w_is_mv = PyMemoryView_Check(w);
3126+
if (w_is_mv) {
31263127
if (BASE_INACCESSIBLE(w)) {
31273128
equal = (v == w);
31283129
goto result;
@@ -3169,7 +3170,7 @@ memory_richcompare(PyObject *v, PyObject *w, int op)
31693170
reshaped during a mixed format comparison loop. */
31703171
// See https://github.com/python/cpython/issues/142663.
31713172
((PyMemoryViewObject *)v)->exports++;
3172-
if (PyMemoryView_Check(w)) {
3173+
if (w_is_mv) {
31733174
((PyMemoryViewObject *)w)->exports++;
31743175
}
31753176

@@ -3191,7 +3192,7 @@ memory_richcompare(PyObject *v, PyObject *w, int op)
31913192
}
31923193

31933194
((PyMemoryViewObject *)v)->exports--;
3194-
if (PyMemoryView_Check(w)) {
3195+
if (w_is_mv) {
31953196
((PyMemoryViewObject *)w)->exports--;
31963197
}
31973198

0 commit comments

Comments
 (0)