kernel: add assert for mis-used k_thread_create()

k_thread_create() works as expected on both uninitialized memory,
or threads that have completely exited.

However, horrible and difficult to comprehend things can happen if a
thread object is already being used by the kernel and
k_thread_create() is called on it.

Historically this has been a problem with test cases trying to be
parsimonious with thread objects and not properly cleaning up
after themselves. Add an assertion for this which should catch
both the illegal creation of a thread already active, or threads
racing to create the same thread object.

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
This commit is contained in:
Andrew Boie 2020-09-05 19:36:08 -07:00 committed by Anas Nashif
commit e0ca403f4c
3 changed files with 22 additions and 0 deletions

View file

@ -516,6 +516,12 @@ struct _thread_base {
#endif
_wait_q_t join_waiters;
#if __ASSERT_ON
/* For detecting calls to k_thread_create() on threads that are
* already active
*/
atomic_t cookie;
#endif
};
typedef struct _thread_base _thread_base_t;

View file

@ -15,6 +15,7 @@
#include <stdbool.h>
#include <kernel_internal.h>
#include <logging/log.h>
#include <sys/atomic.h>
LOG_MODULE_DECLARE(os);
/* Maximum time between the time a self-aborting thread flags itself
@ -603,6 +604,9 @@ void z_thread_single_abort(struct k_thread *thread)
* somewhat undefined behavoir. It must be safe to call
* k_thread_create() or free the object at this point.
*/
#if __ASSERT_ON
atomic_clear(&thread->base.cookie);
#endif
}
if (fn_abort != NULL) {

View file

@ -27,6 +27,7 @@
#include <irq_offload.h>
#include <sys/check.h>
#include <random/rand32.h>
#include <sys/atomic.h>
#define LOG_LEVEL CONFIG_KERNEL_LOG_LEVEL
#include <logging/log.h>
@ -515,6 +516,8 @@ static char *setup_thread_stack(struct k_thread *new_thread,
return stack_ptr;
}
#define THREAD_COOKIE 0x1337C0D3
/*
* The provided stack_size value is presumed to be either the result of
* K_THREAD_STACK_SIZEOF(stack), or the size value passed to the instance
@ -528,6 +531,15 @@ char *z_setup_new_thread(struct k_thread *new_thread,
{
char *stack_ptr;
#if __ASSERT_ON
atomic_val_t old_val = atomic_set(&new_thread->base.cookie,
THREAD_COOKIE);
/* Must be garbage or 0, never already set. Cleared at the end of
* z_thread_single_abort()
*/
__ASSERT(old_val != THREAD_COOKIE,
"re-use of active thread object %p detected", new_thread);
#endif
Z_ASSERT_VALID_PRIO(prio, entry);
#ifdef CONFIG_USERSPACE