Skip to content

Commit de3c806

Browse files
dpgeorgedpgeorge
committed
py: Fix memoryview referencing so it retains ptr to original buffer.
This way, if original parent object is GC'd, the memoryview still points to the underlying buffer data so that buffer is not GC'd.
1 parent c76af32 commit de3c806

2 files changed

Lines changed: 72 additions & 22 deletions

File tree

‎py/objarray.c‎

Lines changed: 54 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,32 @@
3939

4040
#if MICROPY_PY_ARRAY || MICROPY_PY_BUILTINS_BYTEARRAY || MICROPY_PY_BUILTINS_MEMORYVIEW
4141

42+
// About memoryview object: We want to reuse as much code as possible from
43+
// array, and keep the memoryview object 4 words in size so it fits in 1 GC
44+
// block. Also, memoryview must keep a pointer to the base of the buffer so
45+
// that the buffer is not GC'd if the original parent object is no longer
46+
// around (we are assuming that all memoryview'able objects return a pointer
47+
// which points to the start of a GC chunk). Given the above constraints we
48+
// do the following:
49+
// - typecode high bit is set if the buffer is read-write (else read-only)
50+
// - free is the offset in elements to the first item in the memoryview
51+
// - len is the length in elements
52+
// - items points to the start of the original buffer
53+
// Note that we don't handle the case where the original buffer might change
54+
// size due to a resize of the original parent object.
55+
56+
// make (& TYPECODE_MASK) a null operation if memorview not enabled
57+
#if MICROPY_PY_BUILTINS_MEMORYVIEW
58+
#define TYPECODE_MASK (0x7f)
59+
#else
60+
#define TYPECODE_MASK (~(mp_uint_t)1)
61+
#endif
62+
4263
typedef struct _mp_obj_array_t {
4364
mp_obj_base_t base;
4465
mp_uint_t typecode : 8;
4566
// free is number of unused elements after len used elements
4667
// alloc size = len + free
47-
// for memoryview, free=0 is read-only, free=1 is read-write
4868
mp_uint_t free : (8 * sizeof(mp_uint_t) - 8);
4969
mp_uint_t len; // in elements
5070
void *items;
@@ -187,7 +207,7 @@ STATIC mp_obj_t memoryview_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_
187207

188208
// test if the object can be written to
189209
if (mp_get_buffer(args[0], &bufinfo, MP_BUFFER_RW)) {
190-
self->free = 1; // used to indicate writable buffer
210+
self->typecode |= 0x80; // used to indicate writable buffer
191211
}
192212

193213
return self;
@@ -210,7 +230,7 @@ STATIC mp_obj_t array_binary_op(mp_uint_t op, mp_obj_t lhs_in, mp_obj_t rhs_in)
210230
mp_buffer_info_t rhs_bufinfo;
211231
array_get_buffer(lhs_in, &lhs_bufinfo, MP_BUFFER_READ);
212232
if (!mp_get_buffer(rhs_in, &rhs_bufinfo, MP_BUFFER_READ)) {
213-
return mp_const_false;
233+
return mp_const_false;
214234
}
215235
return MP_BOOL(mp_seq_cmp_bytes(op, lhs_bufinfo.buf, lhs_bufinfo.len, rhs_bufinfo.buf, rhs_bufinfo.len));
216236
}
@@ -259,16 +279,16 @@ STATIC mp_obj_t array_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value
259279
"only slices with step=1 (aka None) are supported"));
260280
}
261281
mp_obj_array_t *res;
262-
int sz = mp_binary_get_size('@', o->typecode, NULL);
282+
int sz = mp_binary_get_size('@', o->typecode & TYPECODE_MASK, NULL);
263283
assert(sz > 0);
264284
if (0) {
265285
// dummy
266286
#if MICROPY_PY_BUILTINS_MEMORYVIEW
267287
} else if (o->base.type == &mp_type_memoryview) {
268288
res = m_new_obj(mp_obj_array_t);
269289
*res = *o;
290+
res->free += slice.start;
270291
res->len = slice.stop - slice.start;
271-
res->items += slice.start * sz;
272292
#endif
273293
} else {
274294
res = array_new(o->typecode, slice.stop - slice.start);
@@ -278,18 +298,21 @@ STATIC mp_obj_t array_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value
278298
#endif
279299
} else {
280300
mp_uint_t index = mp_get_index(o->base.type, o->len, index_in, false);
301+
#if MICROPY_PY_BUILTINS_MEMORYVIEW
302+
if (o->base.type == &mp_type_memoryview) {
303+
index += o->free;
304+
if (value != MP_OBJ_SENTINEL && (o->typecode & 0x80) == 0) {
305+
// store to read-only memoryview
306+
return MP_OBJ_NULL;
307+
}
308+
}
309+
#endif
281310
if (value == MP_OBJ_SENTINEL) {
282311
// load
283-
return mp_binary_get_val_array(o->typecode, o->items, index);
312+
return mp_binary_get_val_array(o->typecode & TYPECODE_MASK, o->items, index);
284313
} else {
285314
// store
286-
#if MICROPY_PY_BUILTINS_MEMORYVIEW
287-
if (o->base.type == &mp_type_memoryview && o->free == 0) {
288-
// read-only memoryview
289-
return MP_OBJ_NULL;
290-
}
291-
#endif
292-
mp_binary_set_val_array(o->typecode, o->items, index, value);
315+
mp_binary_set_val_array(o->typecode & TYPECODE_MASK, o->items, index, value);
293316
return mp_const_none;
294317
}
295318
}
@@ -298,15 +321,19 @@ STATIC mp_obj_t array_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value
298321

299322
STATIC mp_int_t array_get_buffer(mp_obj_t o_in, mp_buffer_info_t *bufinfo, mp_uint_t flags) {
300323
mp_obj_array_t *o = o_in;
324+
int sz = mp_binary_get_size('@', o->typecode & TYPECODE_MASK, NULL);
325+
bufinfo->buf = o->items;
326+
bufinfo->len = o->len * sz;
327+
bufinfo->typecode = o->typecode & TYPECODE_MASK;
301328
#if MICROPY_PY_BUILTINS_MEMORYVIEW
302-
if (o->base.type == &mp_type_memoryview && o->free == 0 && (flags & MP_BUFFER_WRITE)) {
303-
// read-only memoryview
304-
return 1;
329+
if (o->base.type == &mp_type_memoryview) {
330+
if ((o->typecode & 0x80) == 0 && (flags & MP_BUFFER_WRITE)) {
331+
// read-only memoryview
332+
return 1;
333+
}
334+
bufinfo->buf += (mp_uint_t)o->free * sz;
305335
}
306336
#endif
307-
bufinfo->buf = o->items;
308-
bufinfo->len = o->len * mp_binary_get_size('@', o->typecode, NULL);
309-
bufinfo->typecode = o->typecode;
310337
return 0;
311338
}
312339

@@ -392,13 +419,14 @@ mp_obj_t mp_obj_new_bytearray_by_ref(mp_uint_t n, void *items) {
392419
typedef struct _mp_obj_array_it_t {
393420
mp_obj_base_t base;
394421
mp_obj_array_t *array;
422+
mp_uint_t offset;
395423
mp_uint_t cur;
396424
} mp_obj_array_it_t;
397425

398426
STATIC mp_obj_t array_it_iternext(mp_obj_t self_in) {
399427
mp_obj_array_it_t *self = self_in;
400428
if (self->cur < self->array->len) {
401-
return mp_binary_get_val_array(self->array->typecode, self->array->items, self->cur++);
429+
return mp_binary_get_val_array(self->array->typecode & TYPECODE_MASK, self->array->items, self->offset + self->cur++);
402430
} else {
403431
return MP_OBJ_STOP_ITERATION;
404432
}
@@ -413,10 +441,14 @@ STATIC const mp_obj_type_t array_it_type = {
413441

414442
STATIC mp_obj_t array_iterator_new(mp_obj_t array_in) {
415443
mp_obj_array_t *array = array_in;
416-
mp_obj_array_it_t *o = m_new_obj(mp_obj_array_it_t);
444+
mp_obj_array_it_t *o = m_new0(mp_obj_array_it_t, 1);
417445
o->base.type = &array_it_type;
418446
o->array = array;
419-
o->cur = 0;
447+
#if MICROPY_PY_BUILTINS_MEMORYVIEW
448+
if (array->base.type == &mp_type_memoryview) {
449+
o->offset = array->free;
450+
}
451+
#endif
420452
return o;
421453
}
422454

‎tests/basics/memoryview_gc.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# test memoryview retains pointer to original object/buffer
2+
3+
b = bytearray(10)
4+
m = memoryview(b)[1:]
5+
for i in range(len(m)):
6+
m[i] = i
7+
8+
# reclaim b, but hopefully not the buffer
9+
b = None
10+
import gc
11+
gc.collect()
12+
13+
# allocate lots of memory
14+
for i in range(100000):
15+
[42, 42, 42, 42]
16+
17+
# check that the memoryview is still what we want
18+
print(list(m))

0 commit comments

Comments
 (0)