Skip to content

Commit c93d9ca

Browse files
dpgeorgedpgeorge
committed
py/gc: Make memory manager and garbage collector thread safe.
By using a single, global mutex, all memory-related functions (alloc, free, realloc, collect, etc) are made thread safe. This means that only one thread can be in such a function at any one time.
1 parent 34fc006 commit c93d9ca

3 files changed

Lines changed: 75 additions & 11 deletions

File tree

‎py/gc.c‎

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,14 @@
9494
#define FTB_CLEAR(block) do { MP_STATE_MEM(gc_finaliser_table_start)[(block) / BLOCKS_PER_FTB] &= (~(1 << ((block) & 7))); } while (0)
9595
#endif
9696

97+
#if MICROPY_PY_THREAD
98+
#define GC_ENTER() mp_thread_mutex_lock(&MP_STATE_MEM(gc_mutex), 1)
99+
#define GC_EXIT() mp_thread_mutex_unlock(&MP_STATE_MEM(gc_mutex))
100+
#else
101+
#define GC_ENTER()
102+
#define GC_EXIT()
103+
#endif
104+
97105
// TODO waste less memory; currently requires that all entries in alloc_table have a corresponding block in pool
98106
void gc_init(void *start, void *end) {
99107
// align end pointer on block boundary
@@ -144,6 +152,10 @@ void gc_init(void *start, void *end) {
144152
// allow auto collection
145153
MP_STATE_MEM(gc_auto_collect_enabled) = 1;
146154

155+
#if MICROPY_PY_THREAD
156+
mp_thread_mutex_init(&MP_STATE_MEM(gc_mutex));
157+
#endif
158+
147159
DEBUG_printf("GC layout:\n");
148160
DEBUG_printf(" alloc table at %p, length " UINT_FMT " bytes, " UINT_FMT " blocks\n", MP_STATE_MEM(gc_alloc_table_start), MP_STATE_MEM(gc_alloc_table_byte_len), MP_STATE_MEM(gc_alloc_table_byte_len) * BLOCKS_PER_ATB);
149161
#if MICROPY_ENABLE_FINALISER
@@ -153,11 +165,15 @@ void gc_init(void *start, void *end) {
153165
}
154166

155167
void gc_lock(void) {
168+
GC_ENTER();
156169
MP_STATE_MEM(gc_lock_depth)++;
170+
GC_EXIT();
157171
}
158172

159173
void gc_unlock(void) {
174+
GC_ENTER();
160175
MP_STATE_MEM(gc_lock_depth)--;
176+
GC_EXIT();
161177
}
162178

163179
bool gc_is_locked(void) {
@@ -236,6 +252,10 @@ STATIC void gc_sweep(void) {
236252
case AT_HEAD:
237253
#if MICROPY_ENABLE_FINALISER
238254
if (FTB_GET(block)) {
255+
#if MICROPY_PY_THREAD
256+
// TODO need to think about reentrancy with finaliser code
257+
assert(!"finaliser with threading not implemented");
258+
#endif
239259
mp_obj_base_t *obj = (mp_obj_base_t*)PTR_FROM_BLOCK(block);
240260
if (obj->type != NULL) {
241261
// if the object has a type then see if it has a __del__ method
@@ -272,7 +292,8 @@ STATIC void gc_sweep(void) {
272292
}
273293

274294
void gc_collect_start(void) {
275-
gc_lock();
295+
GC_ENTER();
296+
MP_STATE_MEM(gc_lock_depth)++;
276297
MP_STATE_MEM(gc_stack_overflow) = 0;
277298
MP_STATE_MEM(gc_sp) = MP_STATE_MEM(gc_stack);
278299
// Trace root pointers. This relies on the root pointers being organised
@@ -294,10 +315,12 @@ void gc_collect_end(void) {
294315
gc_deal_with_stack_overflow();
295316
gc_sweep();
296317
MP_STATE_MEM(gc_last_free_atb_index) = 0;
297-
gc_unlock();
318+
MP_STATE_MEM(gc_lock_depth)--;
319+
GC_EXIT();
298320
}
299321

300322
void gc_info(gc_info_t *info) {
323+
GC_ENTER();
301324
info->total = MP_STATE_MEM(gc_pool_end) - MP_STATE_MEM(gc_pool_start);
302325
info->used = 0;
303326
info->free = 0;
@@ -340,19 +363,23 @@ void gc_info(gc_info_t *info) {
340363

341364
info->used *= BYTES_PER_BLOCK;
342365
info->free *= BYTES_PER_BLOCK;
366+
GC_EXIT();
343367
}
344368

345369
void *gc_alloc(size_t n_bytes, bool has_finaliser) {
346370
size_t n_blocks = ((n_bytes + BYTES_PER_BLOCK - 1) & (~(BYTES_PER_BLOCK - 1))) / BYTES_PER_BLOCK;
347371
DEBUG_printf("gc_alloc(" UINT_FMT " bytes -> " UINT_FMT " blocks)\n", n_bytes, n_blocks);
348372

349-
// check if GC is locked
350-
if (MP_STATE_MEM(gc_lock_depth) > 0) {
373+
// check for 0 allocation
374+
if (n_blocks == 0) {
351375
return NULL;
352376
}
353377

354-
// check for 0 allocation
355-
if (n_blocks == 0) {
378+
GC_ENTER();
379+
380+
// check if GC is locked
381+
if (MP_STATE_MEM(gc_lock_depth) > 0) {
382+
GC_EXIT();
356383
return NULL;
357384
}
358385

@@ -372,13 +399,15 @@ void *gc_alloc(size_t n_bytes, bool has_finaliser) {
372399
if (ATB_3_IS_FREE(a)) { if (++n_free >= n_blocks) { i = i * BLOCKS_PER_ATB + 3; goto found; } } else { n_free = 0; }
373400
}
374401

402+
GC_EXIT();
375403
// nothing found!
376404
if (collected) {
377405
return NULL;
378406
}
379407
DEBUG_printf("gc_alloc(" UINT_FMT "): no free mem, triggering GC\n", n_bytes);
380408
gc_collect();
381409
collected = 1;
410+
GC_ENTER();
382411
}
383412

384413
// found, ending at block i inclusive
@@ -405,6 +434,8 @@ void *gc_alloc(size_t n_bytes, bool has_finaliser) {
405434
ATB_FREE_TO_TAIL(bl);
406435
}
407436

437+
GC_EXIT();
438+
408439
// get pointer to first block
409440
void *ret_ptr = (void*)(MP_STATE_MEM(gc_pool_start) + start_block * BYTES_PER_BLOCK);
410441
DEBUG_printf("gc_alloc(%p)\n", ret_ptr);
@@ -421,7 +452,9 @@ void *gc_alloc(size_t n_bytes, bool has_finaliser) {
421452
// clear type pointer in case it is never set
422453
((mp_obj_base_t*)ret_ptr)->type = NULL;
423454
// set mp_obj flag only if it has a finaliser
455+
GC_ENTER();
424456
FTB_SET(start_block);
457+
GC_EXIT();
425458
}
426459
#else
427460
(void)has_finaliser;
@@ -447,8 +480,10 @@ void *gc_alloc_with_finaliser(mp_uint_t n_bytes) {
447480
// force the freeing of a piece of memory
448481
// TODO: freeing here does not call finaliser
449482
void gc_free(void *ptr) {
483+
GC_ENTER();
450484
if (MP_STATE_MEM(gc_lock_depth) > 0) {
451485
// TODO how to deal with this error?
486+
GC_EXIT();
452487
return;
453488
}
454489

@@ -471,18 +506,25 @@ void gc_free(void *ptr) {
471506
block += 1;
472507
} while (ATB_GET_KIND(block) == AT_TAIL);
473508

509+
GC_EXIT();
510+
474511
#if EXTENSIVE_HEAP_PROFILING
475512
gc_dump_alloc_table();
476513
#endif
477514
} else {
515+
GC_EXIT();
478516
assert(!"bad free");
479517
}
480518
} else if (ptr != NULL) {
519+
GC_EXIT();
481520
assert(!"bad free");
521+
} else {
522+
GC_EXIT();
482523
}
483524
}
484525

485526
size_t gc_nbytes(const void *ptr) {
527+
GC_ENTER();
486528
if (VERIFY_PTR(ptr)) {
487529
size_t block = BLOCK_FROM_PTR(ptr);
488530
if (ATB_GET_KIND(block) == AT_HEAD) {
@@ -491,11 +533,13 @@ size_t gc_nbytes(const void *ptr) {
491533
do {
492534
n_blocks += 1;
493535
} while (ATB_GET_KIND(block + n_blocks) == AT_TAIL);
536+
GC_EXIT();
494537
return n_blocks * BYTES_PER_BLOCK;
495538
}
496539
}
497540

498541
// invalid pointer
542+
GC_EXIT();
499543
return 0;
500544
}
501545

@@ -529,10 +573,6 @@ void *gc_realloc(void *ptr, mp_uint_t n_bytes) {
529573
#else // Alternative gc_realloc impl
530574

531575
void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) {
532-
if (MP_STATE_MEM(gc_lock_depth) > 0) {
533-
return NULL;
534-
}
535-
536576
// check for pure allocation
537577
if (ptr_in == NULL) {
538578
return gc_alloc(n_bytes, false);
@@ -559,6 +599,13 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) {
559599
return NULL;
560600
}
561601

602+
GC_ENTER();
603+
604+
if (MP_STATE_MEM(gc_lock_depth) > 0) {
605+
GC_EXIT();
606+
return NULL;
607+
}
608+
562609
// compute number of new blocks that are requested
563610
size_t new_blocks = (n_bytes + BYTES_PER_BLOCK - 1) / BYTES_PER_BLOCK;
564611

@@ -590,6 +637,7 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) {
590637

591638
// return original ptr if it already has the requested number of blocks
592639
if (new_blocks == n_blocks) {
640+
GC_EXIT();
593641
return ptr_in;
594642
}
595643

@@ -605,6 +653,8 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) {
605653
MP_STATE_MEM(gc_last_free_atb_index) = (block + new_blocks) / BLOCKS_PER_ATB;
606654
}
607655

656+
GC_EXIT();
657+
608658
#if EXTENSIVE_HEAP_PROFILING
609659
gc_dump_alloc_table();
610660
#endif
@@ -620,6 +670,8 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) {
620670
ATB_FREE_TO_TAIL(bl);
621671
}
622672

673+
GC_EXIT();
674+
623675
// zero out the additional bytes of the newly allocated blocks (see comment above in gc_alloc)
624676
memset((byte*)ptr_in + n_bytes, 0, new_blocks * BYTES_PER_BLOCK - n_bytes);
625677

@@ -630,6 +682,8 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) {
630682
return ptr_in;
631683
}
632684

685+
GC_EXIT();
686+
633687
if (!allow_move) {
634688
// not allowed to move memory block so return failure
635689
return NULL;
@@ -666,6 +720,7 @@ void gc_dump_info(void) {
666720
}
667721

668722
void gc_dump_alloc_table(void) {
723+
GC_ENTER();
669724
static const size_t DUMP_BYTES_PER_LINE = 64;
670725
#if !EXTENSIVE_HEAP_PROFILING
671726
// When comparing heap output we don't want to print the starting
@@ -771,6 +826,7 @@ void gc_dump_alloc_table(void) {
771826
mp_printf(&mp_plat_print, "%c", c);
772827
}
773828
mp_print_str(&mp_plat_print, "\n");
829+
GC_EXIT();
774830
}
775831

776832
#if DEBUG_PRINT

‎py/mpstate.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <stdint.h>
3030

3131
#include "py/mpconfig.h"
32+
#include "py/mpthread.h"
3233
#include "py/misc.h"
3334
#include "py/nlr.h"
3435
#include "py/obj.h"
@@ -80,6 +81,11 @@ typedef struct _mp_state_mem_t {
8081
#if MICROPY_PY_GC_COLLECT_RETVAL
8182
size_t gc_collected;
8283
#endif
84+
85+
#if MICROPY_PY_THREAD
86+
// This is a global mutex used to make the GC thread-safe.
87+
mp_thread_mutex_t gc_mutex;
88+
#endif
8389
} mp_state_mem_t;
8490

8591
// This structure hold runtime and VM information. It includes a section

‎py/mpthread.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@
3636
#include <mpthreadport.h>
3737
#endif
3838

39-
mp_state_thread_t *mp_thread_get_state(void);
39+
struct _mp_state_thread_t;
40+
41+
struct _mp_state_thread_t *mp_thread_get_state(void);
4042
void mp_thread_set_state(void *state);
4143
void mp_thread_create(void *(*entry)(void*), void *arg, size_t stack_size);
4244
void mp_thread_mutex_init(mp_thread_mutex_t *mutex);

0 commit comments

Comments
 (0)