K_ESSENTIAL option doesn't have any effect on k_create_thread

[xnaveira]

Describe the bug
In our application we spawn a thread at innit that will be running a loop during all the lifecycle of the app. If that thread stops execution or exits for any reason we want the whole system to restart. To achieve this we configure the thread with the option K_ESSENTIAL which according to the documentation:

This option tags the thread as an essential thread. This instructs the kernel to treat the termination or aborting of the thread as a fatal system error.

What we observe is that configuring the K_ESSENTIAL option has no effect whatsoever and that if the thread stops the rest of the app keeps running.

To Reproduce
Here a simple app to reproduce the problem:

#include <zephyr.h>
#include <sys/printk.h>


#define LOOP2_STACK_SIZE 1024
#define LOOP2_PRIORITY 1

struct k_thread loop2_thread;
K_THREAD_STACK_DEFINE(loop2_area, LOOP2_STACK_SIZE);


void loop2(void *one, void *two, void *three) {
	int i = 0;
	while (1)
	{
		printk("LOOP 2\n");
		i++;
		if (i > 3)
		{
			printk("LOOP 2 EXITING\n");
			return;
		}	
		k_sleep(K_SECONDS(1));
	}
	
}


void main(void)
{
	printk("Hello World! %s\n", CONFIG_BOARD);

        k_tid_t loop2_thread_id = k_thread_create(&loop2_thread, loop2_area,
                                                   K_THREAD_STACK_SIZEOF(loop2_area),
                                                   loop2,
                                                   NULL, NULL, NULL,
                                                   LOOP2_PRIORITY, K_ESSENTIAL, K_NO_WAIT);

	while (1) {
		printk("LOOP 1\n");
		k_sleep(K_SECONDS(2));
	}
	
}

Expected behavior
When the spawned thread exits the app should be restarted (or halted depending on the platform)

Impact
This is a good fit for our application since the devices that are running it are spread geographically and adds robustness to the design.

Environment (please complete the following information):
We have observed this behavior when testing on qemu_cortex as well as on our hardware board which is a Nordic Semiconductors nrf9160 based one.
We are using Nordic SDK 1.9.1 which ships with Zephyr OS v2.7.99

[andyross]

I originally assumed this got dropped in the rework of thread abort that happened about a year ago. But here's the commit that removes the old implementation, and it's not present there either (it's just an assert): c0c8cb0

So I guess we've never done this right per the docs. Oops. Should be a simple fix.

[xnaveira]

So if I understand correctly, this option was dropped? Is there a similar facility in the kernel that we could use?

[andyross]

No, we just never did it the way we documented it. I don't see why we shouldn't match the docs, your use of K_ESSENTIAL as a kind of watchdog feature seems very reasonable to me, and the added code is literally as simple as if(z_is_thread_essential(t)) k_panic(); in the right spot on the abort path.

[xnaveira]

Oh I see. Happy to test a patch if you send it my way :D

[xnaveira]

Is there any work going to be done around this?

[carlescufi]

@andyross could you please open a Pull Request with the fix?

[andyross]

Fix up. Just to quibble with priority though: this is a silly thing to mark "high". The obvious workaround to a lack of system panic on exiting essential threads is just to panic at the end of the thread function. :) Or if you want to catch async aborts, join a thread to it and panic in that.