diff --git a/lib/utils/pyexec.c b/lib/utils/pyexec.c index d1e955d65b9a2..4446b36b61b56 100644 --- a/lib/utils/pyexec.c +++ b/lib/utils/pyexec.c @@ -588,8 +588,8 @@ int pyexec_friendly_repl(void) { // If the GC is locked at this point there is no way out except a reset, // so force the GC to be unlocked to help the user debug what went wrong. - if (MP_STATE_MEM(gc_lock_depth) != 0) { - MP_STATE_MEM(gc_lock_depth) = 0; + if (MP_STATE_THREAD(gc_lock_depth) != 0) { + MP_STATE_THREAD(gc_lock_depth) = 0; } vstr_reset(&line); diff --git a/py/gc.c b/py/gc.c index 53a0d9da4a73c..88adf2045eb42 100644 --- a/py/gc.c +++ b/py/gc.c @@ -150,7 +150,7 @@ void gc_init(void *start, void *end) { MP_STATE_MEM(gc_last_free_atb_index) = 0; // unlock the GC - MP_STATE_MEM(gc_lock_depth) = 0; + MP_STATE_THREAD(gc_lock_depth) = 0; // allow auto collection MP_STATE_MEM(gc_auto_collect_enabled) = 1; @@ -174,19 +174,20 @@ void gc_init(void *start, void *end) { } void gc_lock(void) { - GC_ENTER(); - MP_STATE_MEM(gc_lock_depth)++; - GC_EXIT(); + // This does not need to be atomic or have the GC mutex because: + // - 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)++; } void gc_unlock(void) { - GC_ENTER(); - MP_STATE_MEM(gc_lock_depth)--; - GC_EXIT(); + // This does not need to be atomic, See comment above in gc_lock. + MP_STATE_THREAD(gc_lock_depth)--; } bool gc_is_locked(void) { - return MP_STATE_MEM(gc_lock_depth) != 0; + return MP_STATE_THREAD(gc_lock_depth) != 0; } // ptr should be of type void* @@ -320,7 +321,7 @@ STATIC void gc_sweep(void) { void gc_collect_start(void) { GC_ENTER(); - MP_STATE_MEM(gc_lock_depth)++; + MP_STATE_THREAD(gc_lock_depth)++; #if MICROPY_GC_ALLOC_THRESHOLD MP_STATE_MEM(gc_alloc_amount) = 0; #endif @@ -360,13 +361,13 @@ void gc_collect_end(void) { gc_deal_with_stack_overflow(); gc_sweep(); MP_STATE_MEM(gc_last_free_atb_index) = 0; - MP_STATE_MEM(gc_lock_depth)--; + MP_STATE_THREAD(gc_lock_depth)--; GC_EXIT(); } void gc_sweep_all(void) { GC_ENTER(); - MP_STATE_MEM(gc_lock_depth)++; + MP_STATE_THREAD(gc_lock_depth)++; MP_STATE_MEM(gc_stack_overflow) = 0; gc_collect_end(); } @@ -445,14 +446,13 @@ void *gc_alloc(size_t n_bytes, unsigned int alloc_flags) { return NULL; } - GC_ENTER(); - // check if GC is locked - if (MP_STATE_MEM(gc_lock_depth) > 0) { - GC_EXIT(); + if (MP_STATE_THREAD(gc_lock_depth) > 0) { return NULL; } + GC_ENTER(); + size_t i; size_t end_block; size_t start_block; @@ -573,13 +573,13 @@ void *gc_alloc_with_finaliser(mp_uint_t n_bytes) { // force the freeing of a piece of memory // TODO: freeing here does not call finaliser void gc_free(void *ptr) { - GC_ENTER(); - if (MP_STATE_MEM(gc_lock_depth) > 0) { + if (MP_STATE_THREAD(gc_lock_depth) > 0) { // TODO how to deal with this error? - GC_EXIT(); return; } + GC_ENTER(); + DEBUG_printf("gc_free(%p)\n", ptr); if (ptr == NULL) { @@ -674,15 +674,14 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) { return NULL; } + if (MP_STATE_THREAD(gc_lock_depth) > 0) { + return NULL; + } + void *ptr = ptr_in; GC_ENTER(); - if (MP_STATE_MEM(gc_lock_depth) > 0) { - GC_EXIT(); - return NULL; - } - // get the GC block number corresponding to this pointer assert(VERIFY_PTR(ptr)); size_t block = BLOCK_FROM_PTR(ptr); diff --git a/py/modmicropython.c b/py/modmicropython.c index f7eadf79bd1d2..180f7f186c69a 100644 --- a/py/modmicropython.c +++ b/py/modmicropython.c @@ -130,13 +130,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_MEM(gc_lock_depth)); + return MP_OBJ_NEW_SMALL_INT(MP_STATE_THREAD(gc_lock_depth)); } 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_MEM(gc_lock_depth)); + return MP_OBJ_NEW_SMALL_INT(MP_STATE_THREAD(gc_lock_depth)); } STATIC MP_DEFINE_CONST_FUN_OBJ_0(mp_micropython_heap_locked_obj, mp_micropython_heap_locked); #endif diff --git a/py/modthread.c b/py/modthread.c index 1306dc642b018..64fbb3f198e15 100644 --- a/py/modthread.c +++ b/py/modthread.c @@ -171,6 +171,9 @@ STATIC void *thread_entry(void *args_in) { mp_pystack_init(mini_pystack, &mini_pystack[128]); #endif + // The GC starts off unlocked on this thread. + ts.gc_lock_depth = 0; + // set locals and globals from the calling context mp_locals_set(args->dict_locals); mp_globals_set(args->dict_globals); diff --git a/py/mpstate.h b/py/mpstate.h index 2519c77e2d647..a0e3d4f143096 100644 --- a/py/mpstate.h +++ b/py/mpstate.h @@ -80,7 +80,6 @@ typedef struct _mp_state_mem_t { int gc_stack_overflow; MICROPY_GC_STACK_ENTRY_TYPE gc_stack[MICROPY_ALLOC_GC_STACK_SIZE]; - uint16_t gc_lock_depth; // This variable controls auto garbage collection. If set to 0 then the // GC won't automatically run when gc_alloc can't find enough blocks. But @@ -253,6 +252,9 @@ typedef struct _mp_state_thread_t { uint8_t *pystack_cur; #endif + // Locking of the GC is done per thread. + uint16_t gc_lock_depth; + //////////////////////////////////////////////////////////// // START ROOT POINTER SECTION // Everything that needs GC scanning must start here, and diff --git a/tests/thread/thread_exc1.py b/tests/thread/thread_exc1.py index 16483d7778a06..cd87740929103 100644 --- a/tests/thread/thread_exc1.py +++ b/tests/thread/thread_exc1.py @@ -25,7 +25,12 @@ def thread_entry(): # spawn threads for i in range(n_thread): - _thread.start_new_thread(thread_entry, ()) + while True: + try: + _thread.start_new_thread(thread_entry, ()) + break + except OSError: + pass # busy wait for threads to finish while n_finished < n_thread: diff --git a/tests/thread/thread_exit1.py b/tests/thread/thread_exit1.py index c4a93c45a3df1..186a9be340bd1 100644 --- a/tests/thread/thread_exit1.py +++ b/tests/thread/thread_exit1.py @@ -13,8 +13,13 @@ def thread_entry(): _thread.exit() -_thread.start_new_thread(thread_entry, ()) -_thread.start_new_thread(thread_entry, ()) +for i in range(2): + while True: + try: + _thread.start_new_thread(thread_entry, ()) + break + except OSError: + pass # wait for threads to finish time.sleep(1) diff --git a/tests/thread/thread_exit2.py b/tests/thread/thread_exit2.py index 0cd80e69092a8..5be7945db2205 100644 --- a/tests/thread/thread_exit2.py +++ b/tests/thread/thread_exit2.py @@ -13,8 +13,13 @@ def thread_entry(): raise SystemExit -_thread.start_new_thread(thread_entry, ()) -_thread.start_new_thread(thread_entry, ()) +for i in range(2): + while True: + try: + _thread.start_new_thread(thread_entry, ()) + break + except OSError: + pass # wait for threads to finish time.sleep(1) diff --git a/tests/thread/thread_heap_lock.py b/tests/thread/thread_heap_lock.py new file mode 100644 index 0000000000000..2837e0f3661fd --- /dev/null +++ b/tests/thread/thread_heap_lock.py @@ -0,0 +1,26 @@ +# test interaction of micropython.heap_lock with threads + +import _thread, micropython + +lock1 = _thread.allocate_lock() +lock2 = _thread.allocate_lock() + + +def thread_entry(): + lock1.acquire() + print([1, 2, 3]) + lock2.release() + + +lock1.acquire() +lock2.acquire() + +_thread.start_new_thread(thread_entry, ()) + +micropython.heap_lock() +lock1.release() +lock2.acquire() +micropython.heap_unlock() + +lock1.release() +lock2.release() diff --git a/tests/thread/thread_heap_lock.py.exp b/tests/thread/thread_heap_lock.py.exp new file mode 100644 index 0000000000000..b5d8bb58d9bc3 --- /dev/null +++ b/tests/thread/thread_heap_lock.py.exp @@ -0,0 +1 @@ +[1, 2, 3] diff --git a/tests/thread/thread_stacksize1.py b/tests/thread/thread_stacksize1.py index 5d25509b763a0..cf46b73b770e0 100644 --- a/tests/thread/thread_stacksize1.py +++ b/tests/thread/thread_stacksize1.py @@ -41,7 +41,12 @@ def thread_entry(): # set stack size and spawn a few threads _thread.stack_size(sz) for i in range(n_thread): - _thread.start_new_thread(thread_entry, ()) + while True: + try: + _thread.start_new_thread(thread_entry, ()) + break + except OSError: + pass # reset stack size to default (for subsequent scripts on baremetal) _thread.stack_size() diff --git a/tests/thread/thread_start1.py b/tests/thread/thread_start1.py index f0e696840e556..7274633245a71 100644 --- a/tests/thread/thread_start1.py +++ b/tests/thread/thread_start1.py @@ -18,8 +18,13 @@ def thread_entry(n): foo() -_thread.start_new_thread(thread_entry, (10,)) -_thread.start_new_thread(thread_entry, (20,)) +for i in range(2): + while True: + try: + _thread.start_new_thread(thread_entry, ((i + 1) * 10,)) + break + except OSError: + pass # wait for threads to finish time.sleep(1)