core/msg: prevent thread from stalling indefinitely

core/msg.c

@@ -85,13 +85,7 @@ int msg_try_send(msg_t *m, kernel_pid_t target_pid)
 static int _msg_send(msg_t *m, kernel_pid_t target_pid, bool block,
                      unsigned state)
 {
-#ifdef DEVELHELP
-    if (!pid_is_valid(target_pid)) {
-        DEBUG("msg_send(): target_pid is invalid, continuing anyways\n");
-    }
-#endif /* DEVELHELP */
-
-    thread_t *target = thread_get_unchecked(target_pid);
+    thread_t *target = thread_get(target_pid);
 
     m->sender_pid = thread_getpid();
 
@@ -191,13 +185,7 @@ int msg_send_to_self(msg_t *m)
 
 static int _msg_send_oneway(msg_t *m, kernel_pid_t target_pid)
 {
-#ifdef DEVELHELP
-    if (!pid_is_valid(target_pid)) {
-        DEBUG("%s: target_pid is invalid, continuing anyways\n", __func__);
-    }
-#endif /* DEVELHELP */
-
-    thread_t *target = thread_get_unchecked(target_pid);
+    thread_t *target = thread_get(target_pid);
 
     if (target == NULL) {
         DEBUG("%s: target thread %d does not exist\n", __func__, target_pid);
@@ -272,17 +260,38 @@ int msg_send_bus(msg_t *m, msg_bus_t *bus)
 int msg_send_receive(msg_t *m, msg_t *reply, kernel_pid_t target_pid)
 {
     assert(thread_getpid() != target_pid);
+    if (thread_getpid() == target_pid) {
+        DEBUG("msg_send_receive(): Cannot send and receive on the same thread\n");
+        return -1;
+    }
+
     unsigned state = irq_disable();
     thread_t *me = thread_get_active();
 
+    thread_status_t prev_status = thread_get_status(me);
     sched_set_status(me, STATUS_REPLY_BLOCKED);
     me->wait_data = reply;
 
     /* we reuse (abuse) reply for sending, because wait_data might be
      * overwritten if the target is not in RECEIVE_BLOCKED */
     *reply = *m;
-    /* msg_send blocks until reply received */
-    return _msg_send(reply, target_pid, true, state);
+    /* _msg_send blocks until reply received (except there is an error while sending) */
+    int res = _msg_send(reply, target_pid, true, state);
+
+    if (res == -1) {
+        /* Sending the message failed. We have to restore the previous thread
+         * status, otherwise the thread would remain in a blocked state. */
+
+        /* _msg_send restored interrupts before returning */
+        state = irq_disable();
+
+        me->wait_data = NULL;
+        sched_set_status(me, prev_status);
+
+        irq_restore(state);
+    }
+
+    return res;
 }
 
 int msg_reply(msg_t *m, msg_t *reply)

tests/core/msg_send_receive/Makefile.ci

@@ -4,6 +4,7 @@ BOARD_INSUFFICIENT_MEMORY := \
     arduino-uno \
     atmega328p \
     atmega328p-xplained-mini \
+    atmega8 \
     nucleo-f031k6 \
     nucleo-l011k4 \
     samd10-xmini \

tests/core/msg_send_receive/main.c

@@ -20,6 +20,7 @@
 #include <stdio.h>
 
 #include "cpu_conf.h"
+#include "mutex.h"
 #include "thread.h"
 
 #define THREAD1_STACKSIZE   (THREAD_STACKSIZE_MAIN)
@@ -37,6 +38,8 @@ static kernel_pid_t thread1_pid, thread2_pid;
 static int counter1 = 0;
 static int counter2 = 0;
 
+static mutex_t _mtx = MUTEX_INIT_LOCKED;
+
 static void *thread1(void *args)
 {
     (void)args;
@@ -60,7 +63,7 @@ static void *thread1(void *args)
     }
 
     if (success) {
-        puts("Test successful.");
+        mutex_unlock(&_mtx);
     }
     else {
         puts("Test failed.");
@@ -93,9 +96,44 @@ static void *thread2(void *args)
 
 int main(void)
 {
+    msg_t msg = { 0 };
+
+    /* Test lower PID boundary */
+    int res = msg_send_receive(&msg, &msg, KERNEL_PID_FIRST - 1);
+    if (res != -1) {
+        puts("msg_send_receive() did not return -1 for invalid PID.");
+        return -1;
+    }
+
+    /* Test upper PID boundary */
+    res = msg_send_receive(&msg, &msg, KERNEL_PID_LAST + 1);
+    if (res != -1) {
+        puts("msg_send_receive() did not return -1 for invalid PID.");
+        return -1;
+    }
+
+    /* Check that this thread was not put to @ref STATUS_REPLY_BLOCKED by accident */
+    thread_yield();
+
     thread2_pid = thread_create(thread2_stack, THREAD2_STACKSIZE, THREAD_PRIORITY_MAIN - 2,
                                 0, thread2, NULL, "thread2");
     thread1_pid = thread_create(thread1_stack, THREAD1_STACKSIZE, THREAD_PRIORITY_MAIN - 1,
                                 0, thread1, NULL, "thread1");
+
+    /* Wait for thread1 to unlock the mutex on success */
+    mutex_lock(&_mtx);
+
+    /* Test PID of stopped thread */
+    res = msg_send_receive(&msg, &msg, thread1_pid);
+    if (res != -1) {
+        puts("msg_send_receive() did not return -1 for invalid PID.");
+        return -1;
+    }
+
+    /* Check that this thread was not put to @ref STATUS_REPLY_BLOCKED by accident */
+    thread_yield();
+
+    puts("Test successful.");
+
     return 0;
 }