Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 5 Oct 2023 15:56:04 GMT
From:      Dag-Erling =?utf-8?Q?Sm=C3=B8rgrav?= <des@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 681fbce51d33 - stable/13 - libc: Rewrite quick_exit() and at_quick_exit() using C11 atomics.
Message-ID:  <202310051556.395Fu48F047246@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=681fbce51d337febf945a9f66592711f56cf3f51

commit 681fbce51d337febf945a9f66592711f56cf3f51
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2023-09-26 20:06:27 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2023-10-05 15:55:29 +0000

    libc: Rewrite quick_exit() and at_quick_exit() using C11 atomics.
    
    Compiler memory barriers do not prevent the CPU from executing the code
    out of order.  Switch to C11 atomics.  This also lets us get rid of the
    mutex; instead, loop until the compare_exchange succeeds.
    
    While here, change the return value of at_quick_exit() on failure to
    the more traditional -1, matching atexit().
    
    Sponsored by:   Klara, Inc.
    Reviewed by:    Olivier Certner, kevans, kib
    Differential Revision:  https://reviews.freebsd.org/D41936
    
    (cherry picked from commit 1dc3abb052430279e47c8922d22b30922adcf0f6)
    
    libc: Add a rudimentary test for quick_exit(3).
    
    Sponsored by:   Klara, Inc.
    Reviewed by:    kib
    Differential Revision:  https://reviews.freebsd.org/D41937
    
    (cherry picked from commit c7dd4601aeebbc1bbe131cbe6747476c124b47fe)
---
 lib/libc/stdlib/quick_exit.c            | 40 +++++++---------
 lib/libc/tests/stdlib/Makefile          |  1 +
 lib/libc/tests/stdlib/quick_exit_test.c | 81 +++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+), 24 deletions(-)

diff --git a/lib/libc/stdlib/quick_exit.c b/lib/libc/stdlib/quick_exit.c
index 05db690cb6e7..4dee7b20bd2b 100644
--- a/lib/libc/stdlib/quick_exit.c
+++ b/lib/libc/stdlib/quick_exit.c
@@ -2,6 +2,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  *
  * Copyright (c) 2011 David Chisnall
+ * Copyright (c) 2023 Klara, Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -27,44 +28,35 @@
  */
 
 #include <sys/types.h>
-#include <machine/atomic.h>
+
+#include <stdatomic.h>
 #include <stdlib.h>
-#include <pthread.h>
 
 /**
- * Linked list of quick exit handlers.  This is simpler than the atexit()
- * version, because it is not required to support C++ destructors or
- * DSO-specific cleanups.
+ * Linked list of quick exit handlers.  These will be invoked in reverse
+ * order of insertion when quick_exit() is called.  This is simpler than
+ * the atexit() version, because it is not required to support C++
+ * destructors or DSO-specific cleanups.
  */
 struct quick_exit_handler {
 	struct quick_exit_handler *next;
 	void (*cleanup)(void);
 };
 
-/**
- * Lock protecting the handlers list.
- */
-static pthread_mutex_t atexit_mutex = PTHREAD_MUTEX_INITIALIZER;
-/**
- * Stack of cleanup handlers.  These will be invoked in reverse order when 
- */
-static struct quick_exit_handler *handlers;
+static _Atomic(struct quick_exit_handler *) handlers;
 
 int
 at_quick_exit(void (*func)(void))
 {
 	struct quick_exit_handler *h;
-	
-	h = malloc(sizeof(*h));
 
-	if (NULL == h)
-		return (1);
+	if ((h = calloc(1, sizeof(*h))) == NULL) {
+		return (-1);
+	}
 	h->cleanup = func;
-	pthread_mutex_lock(&atexit_mutex);
-	h->next = handlers;
-	__compiler_membar();
-	handlers = h;
-	pthread_mutex_unlock(&atexit_mutex);
+	while (!atomic_compare_exchange_strong(&handlers, &h->next, h)) {
+		/* nothing */ ;
+	}
 	return (0);
 }
 
@@ -77,8 +69,8 @@ quick_exit(int status)
 	 * XXX: The C++ spec requires us to call std::terminate if there is an
 	 * exception here.
 	 */
-	for (h = handlers; NULL != h; h = h->next) {
-		__compiler_membar();
+	for (h = atomic_load_explicit(&handlers, memory_order_acquire);
+	     h != NULL; h = h->next) {
 		h->cleanup();
 	}
 	_Exit(status);
diff --git a/lib/libc/tests/stdlib/Makefile b/lib/libc/tests/stdlib/Makefile
index 772c470e8865..aa2fa7683cb7 100644
--- a/lib/libc/tests/stdlib/Makefile
+++ b/lib/libc/tests/stdlib/Makefile
@@ -10,6 +10,7 @@ ATF_TESTS_C+=		qsort_b_test
 .endif
 ATF_TESTS_C+=		qsort_r_test
 ATF_TESTS_C+=		qsort_s_test
+ATF_TESTS_C+=		quick_exit_test
 ATF_TESTS_C+=		set_constraint_handler_s_test
 ATF_TESTS_C+=		strfmon_test
 ATF_TESTS_C+=		tsearch_test
diff --git a/lib/libc/tests/stdlib/quick_exit_test.c b/lib/libc/tests/stdlib/quick_exit_test.c
new file mode 100644
index 000000000000..9feed8a6fa63
--- /dev/null
+++ b/lib/libc/tests/stdlib/quick_exit_test.c
@@ -0,0 +1,81 @@
+/*-
+ * Copyright (c) 2023 Klara, Inc.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <sys/wait.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <atf-c.h>
+
+static void func_a(void)
+{
+	if (write(STDOUT_FILENO, "a", 1) != 1)
+		_Exit(1);
+}
+
+static void func_b(void)
+{
+	if (write(STDOUT_FILENO, "b", 1) != 1)
+		_Exit(1);
+}
+
+static void func_c(void)
+{
+	if (write(STDOUT_FILENO, "c", 1) != 1)
+		_Exit(1);
+}
+
+static void child(void)
+{
+	// this will be received by the parent
+	printf("hello, ");
+	fflush(stdout);
+	// this won't, because quick_exit() does not flush
+	printf("world");
+	// these will be called in reverse order, producing "abc"
+	if (at_quick_exit(func_c) != 0 ||
+	    at_quick_exit(func_b) != 0 ||
+	    at_quick_exit(func_a) != 0)
+		_Exit(1);
+	quick_exit(0);
+}
+
+ATF_TC_WITHOUT_HEAD(quick_exit);
+ATF_TC_BODY(quick_exit, tc)
+{
+	char buf[100] = "";
+	ssize_t len;
+	int p[2], wstatus = 0;
+	pid_t pid;
+
+	ATF_REQUIRE(pipe(p) == 0);
+	pid = fork();
+	if (pid == 0) {
+		if (dup2(p[1], STDOUT_FILENO) < 0)
+			_Exit(1);
+		(void)close(p[1]);
+		(void)close(p[0]);
+		child();
+		_Exit(1);
+	}
+	ATF_REQUIRE_MSG(pid > 0,
+	    "expect fork() to succeed");
+	ATF_CHECK_EQ_MSG(pid, waitpid(pid, &wstatus, 0),
+	    "expect to collect child process");
+	ATF_CHECK_EQ_MSG(0, wstatus,
+	    "expect child to exit cleanly");
+	ATF_CHECK_MSG((len = read(p[0], buf, sizeof(buf))) > 0,
+	    "expect to receive output from child");
+	ATF_CHECK_STREQ("hello, abc", buf);
+}
+
+ATF_TP_ADD_TCS(tp)
+{
+	ATF_TP_ADD_TC(tp, quick_exit);
+	return (atf_no_error());
+}



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202310051556.395Fu48F047246>