Skip to content

core/thread: add thread_join() #21572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions core/include/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,22 @@ struct _thread {
#if defined(MODULE_CORE_THREAD_FLAGS) || defined(DOXYGEN)
thread_flags_t flags; /**< currently set flags */
#endif
/**
* @brief Thread exit value or join mutex.
*
* For a thread marked THREAD_CREATE_JOINABLE:
* - status > STATUS_ZOMBIE: address of the joiner signaling mutex, NULL
* if none
* - status == STATUS_ZOMBIE: return value of the thread (not implemented,
* always NULL)
* - status == STATUS_STOPPED: undefined
*
* For a thread NOT marked THREAD_CREATE_JOINABLE:
* - status > STATUS_ZOMBIE: UINTPTR_MAX (marks thread as un-joinable)
* - status == STATUS_ZOMBIE: NULL
* - status == STATUS_STOPPED: undefined
*/
void *exit_val;

clist_node_t rq_entry; /**< run queue entry */

Expand Down Expand Up @@ -239,6 +255,14 @@ struct _thread {
*/
#define THREAD_CREATE_NO_STACKTEST (8)

/**
* @brief Thread is joinable.
*
* On exit, the thread will enter zombie state until @ref thread_join() or @ref
* thread_kill_zombie() is called on it.
*/
#define THREAD_CREATE_JOINABLE (16)

/**
* @brief Legacy flag kept for compatibility.
*
Expand Down Expand Up @@ -284,6 +308,22 @@ kernel_pid_t thread_create(char *stack,
void *arg,
const char *name);

/**
* @brief Wait for a thread to finish.
*
* @pre No other thread is already joining the target thread.
*
* @param[in] pid Thread to join.
* @param[out] exit_value Unimplemented. Set to NULL.
*
* @retval 0 on success.
* @retval -ESRCH if no thread with this PID was found
* @retval -EINVAL if the thread is not joinable, or another thread is
* already joining
* @retval -ECANCELED if the thread was killed while waiting for it to finish
*/
int thread_join(kernel_pid_t pid, void **exit_value);

/**
* @brief Retrieve a thread control block by PID.
* @pre @p pid is valid
Expand Down
9 changes: 8 additions & 1 deletion core/sched.c
Original file line number Diff line number Diff line change
Expand Up @@ -309,13 +309,20 @@ NORETURN void sched_task_exit(void)
DEBUG("sched_task_exit: ending thread %" PRIkernel_pid "...\n",
thread_getpid());

thread_t *me = thread_get_active();
#if defined(MODULE_TEST_UTILS_PRINT_STACK_USAGE) && defined(DEVELHELP)
void print_stack_usage_metric(const char *name, void *stack, unsigned max_size);
thread_t *me = thread_get_active();
print_stack_usage_metric(me->name, me->stack_start, me->stack_size);
#endif

(void)irq_disable();

if (me->exit_val != (void *)UINTPTR_MAX) {
/* this^ && status > THREAD_ZOMBIFY -> joinable */
thread_zombify();
UNREACHABLE();
}

sched_threads[thread_getpid()] = NULL;
sched_num_threads--;

Expand Down
97 changes: 92 additions & 5 deletions core/thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "irq.h"

#include "bitarithm.h"
#include "mutex.h"
#include "sched.h"

#define ENABLE_DEBUG 0
Expand Down Expand Up @@ -68,15 +69,32 @@
return;
}

thread_t *thread = thread_get_active();

irq_disable();
sched_set_status(thread_get_active(), STATUS_ZOMBIE);
sched_set_status(thread, STATUS_ZOMBIE);
if (thread->exit_val && thread->exit_val != (void *)UINTPTR_MAX) {
/* A thread is joining. */
mutex_unlock(thread->exit_val);
}
/* TODO: Here should go the return value of the thread function. */
thread->exit_val = NULL;

irq_enable();
thread_yield_higher();

/* this line should never be reached */
UNREACHABLE();
}

/* Assumes IRQ disabled. */
static void _kill_zombie_unchecked(thread_t *thread)
{
sched_threads[thread->pid] = NULL;
sched_num_threads--;
sched_set_status(thread, STATUS_STOPPED);
}

int thread_kill_zombie(kernel_pid_t pid)
{
DEBUG("thread_kill: Trying to kill PID %" PRIkernel_pid "...\n", pid);
Expand All @@ -92,10 +110,7 @@
else if (thread->status == STATUS_ZOMBIE) {
DEBUG("thread_kill: Thread is a zombie.\n");

sched_threads[pid] = NULL;
sched_num_threads--;
sched_set_status(thread, STATUS_STOPPED);

_kill_zombie_unchecked(thread);
result = 1;
}
else {
Expand Down Expand Up @@ -285,7 +300,7 @@
stacksize = (char *) thread->tls - stack;

_init_tls(thread->tls);
#endif

Check warning on line 303 in core/thread.c

View workflow job for this annotation

GitHub Actions / static-tests

Uncrustify proposes the following patch: --- a/core/thread.c +++ b/core/thread.c @@ -296,8 +296,8 @@ kernel_pid_t thread_create(char *stack, int stacksize, uint8_t priority, * Make sure the TLS area is aligned as required and that the * resulting stack will also be aligned as required */ - thread->tls = (void *) ((uintptr_t) tls & ~ (TLS_ALIGN - 1)); - stacksize = (char *) thread->tls - stack; + thread->tls = (void *)((uintptr_t)tls & ~(TLS_ALIGN - 1)); + stacksize = (char *)thread->tls - stack; _init_tls(thread->tls); #endif

#if defined(DEVELHELP) || defined(SCHED_TEST_STACK) \
|| defined(MODULE_TEST_UTILS_PRINT_STACK_USAGE)
Expand Down Expand Up @@ -325,6 +340,9 @@
}

sched_threads[pid] = thread;
/* UINTPTR_MAX can't be the address of a mutex and marks this thread as not
* joinable. */
thread->exit_val = flags & THREAD_CREATE_JOINABLE ? NULL : (void *)UINTPTR_MAX;

thread->pid = pid;
thread->sp = thread_stack_init(function, arg, stack, stacksize);
Expand Down Expand Up @@ -376,6 +394,75 @@
return pid;
}

int thread_join(kernel_pid_t pid, void **exit_value)
{
(void)exit_value;

mutex_t exit_signal = MUTEX_INIT_LOCKED;
/* Disable interrupts before thread_get() s.t. no thread_kill_zombie() can
* sneak in-between. */
irq_disable();

thread_t *joinee = thread_get(pid);
if (!joinee) {
irq_enable();
/* No thread with this PID was found. */
return -ESRCH;
}

if (joinee->status <= STATUS_ZOMBIE) {
goto finished_checked;
}

if (joinee->exit_val != NULL) {
irq_enable();
/* Another thread is waiting, or not joinable. */
return -EINVAL;
}

joinee->exit_val = &exit_signal;

irq_enable();

mutex_lock(&exit_signal);

irq_disable();

/* The user really shouldn't do it because it counts as multiple joining,
* but it is possible to call thread_kill_zombie() on a ZOMBIE thread
* that had a joiner on it. Precisely, after a thread switches to ZOMBIE but
* before the joiner is woken up. In that case, the thread structure is
* either in an undefined state and we may not touch it anymore (thread_get()
* returns NULL), or the PID already belongs to a new, active thread. There is
* a third possibility that we don't catch here: the PID belongs to a new
* thread, and that one finished too. Then, we just "propagate" this error to the
* next waiter (if any), and the invariant that the thread ended still holds.
* However, the return value of the previous thread is lost and this one is
* unrelated. But for now return values are not implemented anyway (always NULL)
* so it doesn't matter.
*
* TODO: if return values are implemented, either declare the usage of
* thread_kill_zombie() as described above to be UB or find a fix. */
joinee = thread_get(pid);
if (!joinee || joinee->status != STATUS_ZOMBIE) {
irq_enable();
return -ECANCELED;
}

finished_checked:
assume(joinee->status == STATUS_ZOMBIE);

if (exit_value) {
*exit_value = joinee->exit_val;
}

_kill_zombie_unchecked(joinee);

irq_enable();

return 0;
}

static const char *state_names[STATUS_NUMOF] = {
[STATUS_STOPPED] = "stopped",
[STATUS_ZOMBIE] = "zombie",
Expand Down
5 changes: 5 additions & 0 deletions tests/core/thread_join/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
include ../Makefile.core_common

USEMODULE += ztimer_msec

include $(RIOTBASE)/Makefile.include
78 changes: 78 additions & 0 deletions tests/core/thread_join/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Copyright (C) 2025 Mihai Renea <[email protected]>
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @ingroup tests
* @{
*
* @file
* @brief Thread join test application
*
* @author Mihai Renea <[email protected]>
*
* @}
*/

#include <errno.h>
#include <stdio.h>

#include "test_utils/expect.h"
#include "thread.h"
#include "ztimer.h"

static void *joinee_f(void *arg)
{
(void)arg;
ztimer_sleep(ZTIMER_MSEC, 100);

return NULL;
}

static kernel_pid_t _spawn_joinee(bool joinable)
{
static char joinee_stack[THREAD_STACKSIZE_MAIN];
return thread_create(joinee_stack, sizeof(joinee_stack),
THREAD_PRIORITY_MAIN,
joinable ? THREAD_CREATE_JOINABLE : 0,
joinee_f,
NULL,
"joinee");
}

int main(void)
{
puts("START");

kernel_pid_t joinee_pid = _spawn_joinee(true);
expect(joinee_pid >= 0);

int res = thread_join(joinee_pid, NULL);
expect(res == 0);
expect(thread_get(joinee_pid) == NULL);

joinee_pid = _spawn_joinee(true);
expect(joinee_pid >= 0);

ztimer_sleep(ZTIMER_MSEC, 150);

thread_t *joinee = thread_get(joinee_pid);
expect(joinee && joinee->status == STATUS_ZOMBIE);

res = thread_join(joinee_pid, NULL);
expect(res == 0);
expect(thread_get(joinee_pid) == NULL);

joinee_pid = _spawn_joinee(false);

res = thread_join(joinee_pid, NULL);
expect(res == -EINVAL);

puts("SUCCESS");

return 0;
}
13 changes: 13 additions & 0 deletions tests/core/thread_join/tests/01-run.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/usr/bin/env python3

import sys
from testrunner import run


def testfunc(child):
child.expect("START")
child.expect("SUCCESS")


if __name__ == "__main__":
sys.exit(run(testfunc))