exit(3): make it thread-safe

[kib]

Summary

It was explained by Rich Felker <[e-mail]> on libc-coord.
See https://austingroupbugs.net/view.php?id=1845.

[kib]

Add pthread.h.
Describe the behavior in the man page.

[imp]

I love it when a plan comes together so nicely. Agree 100% that this should be deterministic

[imp]

It wouldn't hurt to note in the commit message that this brings us into compliance with posix.1-2024 on this point. Many people may not know why the austin group matters

[kib]

In D46108#1051006, @imp wrote:

It wouldn't hurt to note in the commit message that this brings us into compliance with posix.1-2024 on this point. Many people may not know why the austin group matters

This requirement is definitely not in 1-2024, the issue is new (in the sense that it was created on the austingroup bugtracker recently).

[markj]

lib/libc/stdlib/exit.c

79

We don't care about re-entrancy I suppose? That is, a callback invoked below must not call exit again. If that's expected to "work", i.e., not to deadlock, perhaps the current thread should be allowed to recurse.

[imp]

In D46108#1051011, @kib wrote:

In D46108#1051006, @imp wrote:

It wouldn't hurt to note in the commit message that this brings us into compliance with posix.1-2024 on this point. Many people may not know why the austin group matters

This requirement is definitely not in 1-2024, the issue is new (in the sense that it was created on the austingroup bugtracker recently).

Ah, that's right... I misread the date as last year :(...

[emaste]

lib/libc/stdlib/exit.c

79

Hrm, this seems like a case that could plausibly arise, perhaps accidentally.

[fuz]

lib/libc/stdlib/exit.c

79

I suggest that if we do recurse, exit should crash the program (by means of abort or __builtin_trap()). Crashing seems preferable over the process getting stuck in a deadlock.

[kib]

lib/libc/stdlib/exit.c

79

Yes, re-entrancy is UB already, we call several destructors twice.

There is a discussion on the libc-coord, and the proponent of this change is also against recursion. But if insisted, I can do it.

[markj]

lib/libc/stdlib/exit.c

79

Then the real question is whether recursion actually happens in practice. If it passes an exp-run, then I'd be ok with the patch as it is.

[kib]

lib/libc/stdlib/exit.c

79

What would exp-run test? It is runtime that affected.

In theory some tool/compiler might be the victim, but it is unlikely. I believe that glibc behavior on recursion is SIGSEGV.

[markj]

lib/libc/stdlib/exit.c

79

It's definitely not a comprehensive test, but it still tests quite a lot of code in the ports tree (language runtimes, 3rd party compilers, etc.).

[kib]

lib/libc/stdlib/exit.c

79

Ok, PR 280437

[emaste]

lib/libc/stdlib/exit.c

79

IMO abort or segv is preferable to a hang if this condition were to happen

[kib]

Make the exit mutex recursive.

[brooks]

lib/libc/stdlib/exit.c

79

Then the real question is whether recursion actually happens in practice. If it passes an exp-run, then I'd be ok with the patch as it is.

https://austingroupbugs.net/view.php?id=1845#c6847 says that there are real world cases so glibc and bionic will be supporting recursion which almost certainly means we should even though it's an obviously terrible idea.

[kib]

Below are two test programs I used to verify the patch:

exit code should be 12

#include <stdlib.h>

static void
myatexit(void)
{
	exit(12);
}

int
main(void)
{
	atexit(myatexit);
	exit(1);
}

exit code should be 12, program must not hang or crash

#include <pthread.h>
#include <stdlib.h>
#include <unistd.h>

static void
myatexit(void)
{
	sleep(10);
	exit(12);
}

static void *
mythreadexit(void *)
{
	sleep(5);
	exit(15);
	return (NULL);
}

int
main(void)
{
	pthread_t thr;

	atexit(myatexit);
	pthread_create(&thr, NULL, mythreadexit, NULL);
	exit(1);
}

[markj]

In D46108#1052369, @kib wrote:

Below are two test programs I used to verify the patch:

I converted these into ATF tests, with a bit of modification: https://reviews.freebsd.org/D46176