Skip to content

core/msg: prevent thread from stalling indefinitely#22193

Open
HendrikVE wants to merge 3 commits intoRIOT-OS:masterfrom
HendrikVE:hendrik/core/msg/fix-stalling-thread
Open

core/msg: prevent thread from stalling indefinitely#22193
HendrikVE wants to merge 3 commits intoRIOT-OS:masterfrom
HendrikVE:hendrik/core/msg/fix-stalling-thread

Conversation

@HendrikVE
Copy link
Copy Markdown
Contributor

Contribution description

Before, when calling msg_send_receive() with an invalid target_pid the thread trying to send the message would have been put into blocked state. But when sending the message is failing, this thread will then remain blocked forever, since it can't expect to receive a response from a thread that never got its message (because this thread simply does not exist).

Testing procedure

All you need to do is to call msg_send_receive with target_pid=0.

Declaration of AI-Tools / LLMs usage:

AI-Tools / LLMs that were used are:

  • none

@HendrikVE HendrikVE requested a review from kaspar030 as a code owner April 14, 2026 15:36
@github-actions github-actions Bot added the Area: core Area: RIOT kernel. Handle PRs marked with this with care! label Apr 14, 2026
@HendrikVE HendrikVE requested review from benpicco and maribu April 14, 2026 15:36
@crasbe crasbe added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs >1 ACK Integration Process: This PR requires more than one ACK labels Apr 14, 2026
@github-actions github-actions Bot added the Process: missing approvals Integration Process: PR needs more ACKS (handled by action) label Apr 14, 2026
Comment thread core/msg.c Outdated
Comment thread core/msg.c Outdated
@riot-ci
Copy link
Copy Markdown

riot-ci commented Apr 14, 2026

Murdock results

FAILED

78965c6 tests/core/msg_send_receive: exclude atmega8

Build failures (1)
Application Target Toolchain Runtime (s) Worker
examples/networking/dtls/dtls-wolfssl cc1350-launchpad gnu 7.79 flatserv

Artifacts

@HendrikVE HendrikVE force-pushed the hendrik/core/msg/fix-stalling-thread branch from 1bc42a8 to 3d1fe44 Compare April 16, 2026 12:07
Copy link
Copy Markdown
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@HendrikVE
Copy link
Copy Markdown
Contributor Author

@maribu Would you mind adding the second ACK? :)

@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Apr 20, 2026

I wonder if it would be worth adding a test case for this?

@HendrikVE
Copy link
Copy Markdown
Contributor Author

I wonder if it would be worth adding a test case for this?

Makes sense to me. I'd try to integrate it into the existing test at tests/core/msg_send_receive/main.c

@github-actions github-actions Bot added the Area: tests Area: tests and testing framework label Apr 27, 2026
@HendrikVE
Copy link
Copy Markdown
Contributor Author

I added some checks to the existing code. The test now also checks for the return value of msg_send_receive and it makes sure that the main thread is still scheduled by calling thread_yield. If it was blocked due to the bug the final puts("Test successful."); line would not get executed.

@kfessel kfessel changed the title core/msg.c: prevent thread from stalling indefinitely core/msg: prevent thread from stalling indefinitely Apr 28, 2026
@HendrikVE HendrikVE force-pushed the hendrik/core/msg/fix-stalling-thread branch from 5581f80 to 85b0fd0 Compare April 28, 2026 11:14
@HendrikVE HendrikVE force-pushed the hendrik/core/msg/fix-stalling-thread branch from 85b0fd0 to 77a6f9f Compare April 28, 2026 13:26
@benpicco benpicco removed the Process: missing approvals Integration Process: PR needs more ACKS (handled by action) label Apr 29, 2026
@benpicco benpicco enabled auto-merge April 29, 2026 16:55
@benpicco benpicco added this pull request to the merge queue Apr 29, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 29, 2026
@maribu maribu added this pull request to the merge queue Apr 30, 2026
@maribu maribu removed this pull request from the merge queue due to a manual request Apr 30, 2026
@maribu
Copy link
Copy Markdown
Member

maribu commented Apr 30, 2026

The CI failure is not a glitch, sadly

@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Apr 30, 2026

The CI failure is not a glitch, sadly

No, it's the classic "changed a test, now it doesn't fit into the memory of $smöl_microcontroller anymore".

I ran make generate-Makefile.ci on skyleaf and this is the result:

buechse@skyleaf:~/RIOTstuff/riot-vanillaice/RIOT/tests/core/msg_send_receive$ git diff Makefile.ci
diff --git a/tests/core/msg_send_receive/Makefile.ci b/tests/core/msg_send_receive/Makefile.ci
index d28f5b77f9..5f2032215b 100644
--- a/tests/core/msg_send_receive/Makefile.ci
+++ b/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 \

@HendrikVE
Copy link
Copy Markdown
Contributor Author

Do I get this right that the solution is to add the atmega8 to the list? ^^

@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Apr 30, 2026

Yes.

@crasbe crasbe enabled auto-merge April 30, 2026 12:12
@crasbe crasbe added this pull request to the merge queue Apr 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 30, 2026
@crasbe crasbe added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: full build disable CI build filter CI: no fast fail don't abort PR build after first error and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 30, 2026
@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Apr 30, 2026

Fascinating. I ran make generate-Makefile.ci locally on skyleaf and it did not add cc1350-launchpad to the examples/networking/dtls/dtls-wolfssl/Makefile.ci...

Considering the build failed twice because of it, it might be a good idea to do it nevertheless :D

But the good news is that no other application has to be modified.

@crasbe crasbe removed the CI: full build disable CI build filter label Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: tests Area: tests and testing framework CI: no fast fail don't abort PR build after first error CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs >1 ACK Integration Process: This PR requires more than one ACK Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants