py/gc: Allow gc_free from inside a gc_sweep finalizer.

Do this by tracking being inside gc collection with a
separate flag, GC_COLLECT_FLAG. In gc_free(),
ignore this flag when determining if the heap is locked.

* For finalisers calling gc_free() when heap is otherwise unlocked,
  this allows memory to be immediately freed (potentially
  avoiding a MemoryError).

* Hard IRQs still can't call gc_free(), as heap will be locked via
  gc_lock().

* If finalisers are disabled then all of this code can be compiled
  out to save some code size.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
This commit is contained in:
Angus Gratton
2024-12-04 10:58:06 +11:00
committed by Damien George
parent 8a2ff2ca73
commit 40e1c111e1
3 changed files with 36 additions and 16 deletions

35
py/gc.c
View File

@@ -334,12 +334,12 @@ void gc_lock(void) {
// - each thread has its own gc_lock_depth so there are no races between threads;
// - a hard interrupt will only change gc_lock_depth during its execution, and
// upon return will restore the value of gc_lock_depth.
MP_STATE_THREAD(gc_lock_depth)++;
MP_STATE_THREAD(gc_lock_depth) += (1 << GC_LOCK_DEPTH_SHIFT);
}
void gc_unlock(void) {
// This does not need to be atomic, See comment above in gc_lock.
MP_STATE_THREAD(gc_lock_depth)--;
MP_STATE_THREAD(gc_lock_depth) -= (1 << GC_LOCK_DEPTH_SHIFT);
}
bool gc_is_locked(void) {
@@ -581,13 +581,18 @@ static void gc_sweep(void) {
}
}
void gc_collect_start(void) {
static void gc_collect_start_common(void) {
GC_ENTER();
MP_STATE_THREAD(gc_lock_depth)++;
assert((MP_STATE_THREAD(gc_lock_depth) & GC_COLLECT_FLAG) == 0);
MP_STATE_THREAD(gc_lock_depth) |= GC_COLLECT_FLAG;
MP_STATE_MEM(gc_stack_overflow) = 0;
}
void gc_collect_start(void) {
gc_collect_start_common();
#if MICROPY_GC_ALLOC_THRESHOLD
MP_STATE_MEM(gc_alloc_amount) = 0;
#endif
MP_STATE_MEM(gc_stack_overflow) = 0;
// Trace root pointers. This relies on the root pointers being organised
// correctly in the mp_state_ctx structure. We scan nlr_top, dict_locals,
@@ -658,14 +663,12 @@ void gc_collect_end(void) {
for (mp_state_mem_area_t *area = &MP_STATE_MEM(area); area != NULL; area = NEXT_AREA(area)) {
area->gc_last_free_atb_index = 0;
}
MP_STATE_THREAD(gc_lock_depth)--;
MP_STATE_THREAD(gc_lock_depth) &= ~GC_COLLECT_FLAG;
GC_EXIT();
}
void gc_sweep_all(void) {
GC_ENTER();
MP_STATE_THREAD(gc_lock_depth)++;
MP_STATE_MEM(gc_stack_overflow) = 0;
gc_collect_start_common();
gc_collect_end();
}
@@ -902,10 +905,13 @@ found:
// force the freeing of a piece of memory
// TODO: freeing here does not call finaliser
void gc_free(void *ptr) {
if (MP_STATE_THREAD(gc_lock_depth) > 0) {
// Cannot free while the GC is locked. However free is an optimisation
// to reclaim the memory immediately, this means it will now be left
// until the next collection.
// Cannot free while the GC is locked, unless we're only doing a gc sweep.
// However free is an optimisation to reclaim the memory immediately, this
// means it will now be left until the next collection.
//
// (We have the optimisation to free immediately from inside a gc sweep so
// that finalisers can free more memory when trying to avoid MemoryError.)
if (MP_STATE_THREAD(gc_lock_depth) & ~GC_COLLECT_FLAG) {
return;
}
@@ -930,7 +936,8 @@ void gc_free(void *ptr) {
#endif
size_t block = BLOCK_FROM_PTR(area, ptr);
assert(ATB_GET_KIND(area, block) == AT_HEAD);
assert(ATB_GET_KIND(area, block) == AT_HEAD
|| (ATB_GET_KIND(area, block) == AT_MARK && (MP_STATE_THREAD(gc_lock_depth) & GC_COLLECT_FLAG)));
#if MICROPY_ENABLE_FINALISER
FTB_CLEAR(area, block);

View File

@@ -132,13 +132,13 @@ static MP_DEFINE_CONST_FUN_OBJ_0(mp_micropython_heap_lock_obj, mp_micropython_he
static mp_obj_t mp_micropython_heap_unlock(void) {
gc_unlock();
return MP_OBJ_NEW_SMALL_INT(MP_STATE_THREAD(gc_lock_depth));
return MP_OBJ_NEW_SMALL_INT(MP_STATE_THREAD(gc_lock_depth) >> GC_LOCK_DEPTH_SHIFT);
}
static MP_DEFINE_CONST_FUN_OBJ_0(mp_micropython_heap_unlock_obj, mp_micropython_heap_unlock);
#if MICROPY_PY_MICROPYTHON_HEAP_LOCKED
static mp_obj_t mp_micropython_heap_locked(void) {
return MP_OBJ_NEW_SMALL_INT(MP_STATE_THREAD(gc_lock_depth));
return MP_OBJ_NEW_SMALL_INT(MP_STATE_THREAD(gc_lock_depth) >> GC_LOCK_DEPTH_SHIFT);
}
static MP_DEFINE_CONST_FUN_OBJ_0(mp_micropython_heap_locked_obj, mp_micropython_heap_locked);
#endif

View File

@@ -77,6 +77,18 @@ typedef struct _mp_sched_item_t {
mp_obj_t arg;
} mp_sched_item_t;
// gc_lock_depth field is a combination of the GC_COLLECT_FLAG
// bit and a lock depth shifted GC_LOCK_DEPTH_SHIFT bits left.
#if MICROPY_ENABLE_FINALISER
#define GC_COLLECT_FLAG 1
#define GC_LOCK_DEPTH_SHIFT 1
#else
// If finalisers are disabled then this check doesn't matter, as gc_lock()
// is called anywhere else that heap can't be changed. So save some code size.
#define GC_COLLECT_FLAG 0
#define GC_LOCK_DEPTH_SHIFT 0
#endif
// This structure holds information about a single contiguous area of
// memory reserved for the memory manager.
typedef struct _mp_state_mem_area_t {
@@ -268,6 +280,7 @@ typedef struct _mp_state_thread_t {
#endif
// Locking of the GC is done per thread.
// See GC_LOCK_DEPTH_SHIFT for an explanation of this field.
uint16_t gc_lock_depth;
////////////////////////////////////////////////////////////