micropython · dpgeorge · May 10, 2021 · May 4, 2021 · May 10, 2021
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1 @@
+[1, 2, 3]
diff --git 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
@@ -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)