Fix a concurrency bug in OP_WAIT_TIMEOUT

pguyot · pguyot · commit fea485cc6221 · 2026-02-17T13:54:08.000+01:00
Fix a bug that could explain some CI flappiness. If a message arrived before
first execution of op_wait_timeout, the timer would be configured and the
wait would time out unless another message arrived.

Fix another bug where a process doing receive after Timeout -&gt; Expr end
would loop forever if a message was in the mailbox.

Signed-off-by: Paul Guyot &lt;pguyot@kallisys.net&gt;
diff --git a/src/libAtomVM/jit.c b/src/libAtomVM/jit.c
@@ -888,6 +888,7 @@ static Context *jit_process_signal_messages(Context *ctx, JITState *jit_state)
 static term jit_mailbox_peek(Context *ctx)
 {
     TRACE("jit_mailbox_peek: ctx->process_id=%" PRId32 "\n", ctx->process_id);
+    ctx->mailbox.receive_has_match_clauses = true;
     term out = term_invalid_term();
     mailbox_peek(ctx, &out);
     return out;
@@ -897,12 +898,14 @@ static void jit_mailbox_remove_message(Context *ctx)
 {
     TRACE("jit_mailbox_remove_message: ctx->process_id=%" PRId32 "\n", ctx->process_id);
     mailbox_remove_message(&ctx->mailbox, &ctx->heap);
+    ctx->mailbox.receive_has_match_clauses = false;
 }
 
 static void jit_timeout(Context *ctx)
 {
     TRACE("jit_timeout: ctx->process_id=%" PRId32 "\n", ctx->process_id);
     context_update_flags(ctx, ~WaitingTimeoutExpired, NoFlags);
+    ctx->mailbox.receive_has_match_clauses = false;
     mailbox_reset(&ctx->mailbox);
 }
 
@@ -952,18 +955,23 @@ static Context *jit_wait_timeout(Context *ctx, JITState *jit_state, term timeout
         needs_to_wait = 1;
     } else if (context_get_flags(ctx, WaitingTimeout) != 0) {
         needs_to_wait = 1;
-    } else if (!mailbox_has_next(&ctx->mailbox)) {
-        needs_to_wait = 1;
     }
+    // else: WaitingTimeoutExpired -- fall through to timeout.
+    // Any messages in the mailbox are left for the next receive.
 
     if (needs_to_wait) {
+        // Signal processing may have moved messages to the inner list.
+        // If there are match clauses (loop_rec was executed), jump to
+        // loop_rec to scan them.
+        if (ctx->mailbox.receive_has_match_clauses && mailbox_has_next(&ctx->mailbox)) {
+            jit_state->continuation = module_get_native_entry_point(jit_state->module, label);
+            return ctx;
+        }
         return jit_schedule_wait_cp(ctx, jit_state);
-    } else {
-        // clang cannot tail-optimize this, so return to loop to avoid any stack overflow
-        // __attribute__((musttail)) return jit_state->continuation(ctx, jit_state, &module_native_interface);
-        jit_state->continuation = module_get_native_entry_point(jit_state->module, label);
-        return ctx;
     }
+    // else: timer expired, fall through to timeout
+    // jit_state->continuation already points to the trap handler code
+    return ctx;
 }
 
 static Context *jit_wait_timeout_trap_handler(Context *ctx, JITState *jit_state, int label)
@@ -976,6 +984,7 @@ static Context *jit_wait_timeout_trap_handler(Context *ctx, JITState *jit_state,
         return scheduler_wait(ctx);
     }
 
+    // Messages available, jump to loop_rec to scan them.
     jit_state->continuation = module_get_native_entry_point(jit_state->module, label);
     return ctx;
 }
diff --git a/src/libAtomVM/mailbox.c b/src/libAtomVM/mailbox.c
@@ -45,6 +45,7 @@ void mailbox_init(Mailbox *mbx)
     mbx->inner_last = NULL;
     mbx->receive_pointer = NULL;
     mbx->receive_pointer_prev = NULL;
+    mbx->receive_has_match_clauses = false;
 }
 
 // Convert a mailbox message (struct Message or struct TermSignal) to a heap
diff --git a/src/libAtomVM/mailbox.h b/src/libAtomVM/mailbox.h
@@ -178,6 +178,10 @@ typedef struct
     // Receive pointers are on inner list items.
     MailboxMessage *receive_pointer;
     MailboxMessage *receive_pointer_prev;
+    // Set by loop_rec, cleared by remove_message and timeout.
+    // Used by wait_timeout to know if there are match clauses to try
+    // when signal processing moved messages to the inner list.
+    bool receive_has_match_clauses;
 } Mailbox;
 
 // TODO: a lot of this code depends on Context * and should be decoupled
diff --git a/src/libAtomVM/opcodesswitch.h b/src/libAtomVM/opcodesswitch.h
@@ -2798,6 +2798,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
                     }
                     PROCESS_SIGNAL_MESSAGES();
                     mailbox_remove_message(&ctx->mailbox, &ctx->heap);
+                    ctx->mailbox.receive_has_match_clauses = false;
                     // Cannot GC now as remove_message is GC neutral
                 #endif
                 break;
@@ -2808,6 +2809,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
 
                 #ifdef IMPL_EXECUTE_LOOP
                     context_update_flags(ctx, ~WaitingTimeoutExpired, NoFlags);
+                    ctx->mailbox.receive_has_match_clauses = false;
 
                     mailbox_reset(&ctx->mailbox);
                 #endif
@@ -2823,6 +2825,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
                 TRACE("loop_rec/2, dreg=%c%i\n", T_DEST_REG(dreg));
 
                 #ifdef IMPL_EXECUTE_LOOP
+                    ctx->mailbox.receive_has_match_clauses = true;
                     term ret;
                     PROCESS_SIGNAL_MESSAGES();
                     if (mailbox_peek(ctx, &ret)) {
@@ -2898,15 +2901,19 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
                         needs_to_wait = 1;
                     } else if (context_get_flags(ctx, WaitingTimeout) != 0) {
                         needs_to_wait = 1;
-                    } else if (!mailbox_has_next(&ctx->mailbox)) {
-                        needs_to_wait = 1;
                     }
+                    // else: WaitingTimeoutExpired -- fall through to timeout.
+                    // Any messages in the mailbox are left for the next receive.
 
                     if (needs_to_wait) {
+                        // Signal processing may have moved messages to the inner
+                        // list. If there are match clauses (loop_rec was
+                        // executed), jump to loop_rec to scan them.
+                        if (ctx->mailbox.receive_has_match_clauses && mailbox_has_next(&ctx->mailbox)) {
+                            JUMP_TO_ADDRESS(mod->labels[label]);
+                        }
                         ctx->waiting_with_timeout = true;
                         SCHEDULE_WAIT(mod, saved_pc);
-                    } else {
-                        JUMP_TO_ADDRESS(mod->labels[label]);
                     }
                 #endif
 
@@ -2922,8 +2929,6 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
 #ifdef IMPL_EXECUTE_LOOP
 wait_timeout_trap_handler:
             {
-                // Determine if a message arrived to either jump to timeout label
-                // or to continuation.
                 // Redo the offset computation and refetch the label
                 int label;
                 DECODE_LABEL(label, pc)
@@ -2932,6 +2937,8 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
                 TRACE("wait_timeout_trap_handler, label: %i\n", label);
                 PROCESS_SIGNAL_MESSAGES();
                 if (context_get_flags(ctx, WaitingTimeoutExpired)) {
+                    // Timer expired -- fall through to timeout.
+                    // Any messages in the mailbox are left for the next receive.
                     ctx->waiting_with_timeout = false;
                 } else {
                     if (UNLIKELY(!mailbox_has_next(&ctx->mailbox))) {

Original file line number	Diff line number	Diff line change
`@@ -888,6 +888,7 @@ static Context jit_process_signal_messages(Context ctx, JITState *jit_state)`
`888`	`888`	`static term jit_mailbox_peek(Context *ctx)`
`889`	`889`	`{`
`890`	`890`	`TRACE("jit_mailbox_peek: ctx->process_id=%" PRId32 "\n", ctx->process_id);`
	`891`	`+ ctx->mailbox.receive_has_match_clauses = true;`
`891`	`892`	`term out = term_invalid_term();`
`892`	`893`	`mailbox_peek(ctx, &out);`
`893`	`894`	`return out;`
`@@ -897,12 +898,14 @@ static void jit_mailbox_remove_message(Context *ctx)`
`897`	`898`	`{`
`898`	`899`	`TRACE("jit_mailbox_remove_message: ctx->process_id=%" PRId32 "\n", ctx->process_id);`
`899`	`900`	`mailbox_remove_message(&ctx->mailbox, &ctx->heap);`
	`901`	`+ ctx->mailbox.receive_has_match_clauses = false;`
`900`	`902`	`}`
`901`	`903`
`902`	`904`	`static void jit_timeout(Context *ctx)`
`903`	`905`	`{`
`904`	`906`	`TRACE("jit_timeout: ctx->process_id=%" PRId32 "\n", ctx->process_id);`
`905`	`907`	`context_update_flags(ctx, ~WaitingTimeoutExpired, NoFlags);`
	`908`	`+ ctx->mailbox.receive_has_match_clauses = false;`
`906`	`909`	`mailbox_reset(&ctx->mailbox);`
`907`	`910`	`}`
`908`	`911`
`@@ -952,18 +955,23 @@ static Context jit_wait_timeout(Context ctx, JITState *jit_state, term timeout`
`952`	`955`	`needs_to_wait = 1;`
`953`	`956`	`} else if (context_get_flags(ctx, WaitingTimeout) != 0) {`
`954`	`957`	`needs_to_wait = 1;`
`955`		`- } else if (!mailbox_has_next(&ctx->mailbox)) {`
`956`		`- needs_to_wait = 1;`
`957`	`958`	`}`
	`959`	`+ // else: WaitingTimeoutExpired -- fall through to timeout.`
	`960`	`+ // Any messages in the mailbox are left for the next receive.`
`958`	`961`
`959`	`962`	`if (needs_to_wait) {`
	`963`	`+ // Signal processing may have moved messages to the inner list.`
	`964`	`+ // If there are match clauses (loop_rec was executed), jump to`
	`965`	`+ // loop_rec to scan them.`
	`966`	`+ if (ctx->mailbox.receive_has_match_clauses && mailbox_has_next(&ctx->mailbox)) {`
	`967`	`+ jit_state->continuation = module_get_native_entry_point(jit_state->module, label);`
	`968`	`+ return ctx;`
	`969`	`+ }`
`960`	`970`	`return jit_schedule_wait_cp(ctx, jit_state);`
`961`		`- } else {`
`962`		`- // clang cannot tail-optimize this, so return to loop to avoid any stack overflow`
`963`		`- // __attribute__((musttail)) return jit_state->continuation(ctx, jit_state, &module_native_interface);`
`964`		`- jit_state->continuation = module_get_native_entry_point(jit_state->module, label);`
`965`		`- return ctx;`
`966`	`971`	`}`
	`972`	`+ // else: timer expired, fall through to timeout`
	`973`	`+ // jit_state->continuation already points to the trap handler code`
	`974`	`+ return ctx;`
`967`	`975`	`}`
`968`	`976`
`969`	`977`	`static Context jit_wait_timeout_trap_handler(Context ctx, JITState *jit_state, int label)`
`@@ -976,6 +984,7 @@ static Context jit_wait_timeout_trap_handler(Context ctx, JITState *jit_state,`
`976`	`984`	`return scheduler_wait(ctx);`
`977`	`985`	`}`
`978`	`986`
	`987`	`+ // Messages available, jump to loop_rec to scan them.`
`979`	`988`	`jit_state->continuation = module_get_native_entry_point(jit_state->module, label);`
`980`	`989`	`return ctx;`
`981`	`990`	`}`
Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,7 @@ void mailbox_init(Mailbox *mbx)`
`45`	`45`	`mbx->inner_last = NULL;`
`46`	`46`	`mbx->receive_pointer = NULL;`
`47`	`47`	`mbx->receive_pointer_prev = NULL;`
	`48`	`+ mbx->receive_has_match_clauses = false;`
`48`	`49`	`}`
`49`	`50`
`50`	`51`	`// Convert a mailbox message (struct Message or struct TermSignal) to a heap`