[patch] Fix for bz14333 -- race between atexit() and exit()
Paul Pluzhnikov
ppluzhnikov@google.com
Tue Jul 11 00:53:00 GMT 2017
Greetings,
This patch is an update to previous patch by Anoop:
https://sourceware.org/ml/libc-alpha/2008-09/msg00013.html
See discussion in the above message.
Changes from the previous patch: I extended the lifetime of the lock
from __new_exitfn to its callers, since they *also* modify global
state.
Tested: "make check" on Linux/x86_64.
Also ran the test from
https://sourceware.org/bugzilla/show_bug.cgi?id=14333#c5 a few hundred
times.
2017-07-10 Paul Pluzhnikov <ppluzhnikov@google.com>
Anoop V Chakkalakkal <anoop.vijayan@in.ibm.com>
[BZ #14333]
* stdlib/cxa_atexit.c (__internal_atexit): Use __exit_funcs_lock.
(__new_exitfn): Fail registration when we finished at_exit processing.
* stdlib/on_exit.c (__on_exit): Likewise.
* stdlib/exit.c (__exit_funcs_done): New variable.
(__run_exit_handlers): Use __exit_funcs_lock.
* stdlib/exit.h (__exit_funcs_lock, __exit_funcs_done): New
declarations.
--
Paul Pluzhnikov
-------------- next part --------------
diff --git a/stdlib/cxa_atexit.c b/stdlib/cxa_atexit.c
index ce5d9f22b4..5db0b33060 100644
--- a/stdlib/cxa_atexit.c
+++ b/stdlib/cxa_atexit.c
@@ -26,16 +26,25 @@
#undef __cxa_atexit
+/* We change global data, so we need locking. */
+__libc_lock_define_initialized (, __exit_funcs_lock)
+
int
attribute_hidden
__internal_atexit (void (*func) (void *), void *arg, void *d,
struct exit_function_list **listp)
{
- struct exit_function *new = __new_exitfn (listp);
+ struct exit_function *new;
+
+ __libc_lock_lock (__exit_funcs_lock);
+ new = __new_exitfn (listp);
if (new == NULL)
- return -1;
+ {
+ __libc_lock_unlock (__exit_funcs_lock);
+ return -1;
+ }
#ifdef PTR_MANGLE
PTR_MANGLE (func);
@@ -45,6 +54,7 @@ __internal_atexit (void (*func) (void *), void *arg, void *d,
new->func.cxa.dso_handle = d;
atomic_write_barrier ();
new->flavor = ef_cxa;
+ __libc_lock_unlock (__exit_funcs_lock);
return 0;
}
@@ -60,10 +70,6 @@ __cxa_atexit (void (*func) (void *), void *arg, void *d)
libc_hidden_def (__cxa_atexit)
-/* We change global data, so we need locking. */
-__libc_lock_define_initialized (static, lock)
-
-
static struct exit_function_list initial;
struct exit_function_list *__exit_funcs = &initial;
uint64_t __new_exitfn_called;
@@ -76,7 +82,10 @@ __new_exitfn (struct exit_function_list **listp)
struct exit_function *r = NULL;
size_t i = 0;
- __libc_lock_lock (lock);
+ if (__exit_funcs_done == 1)
+ /* exit code finished processing all handlers
+ so fail this registration */
+ return NULL;
for (l = *listp; l != NULL; p = l, l = l->next)
{
@@ -127,7 +136,5 @@ __new_exitfn (struct exit_function_list **listp)
++__new_exitfn_called;
}
- __libc_lock_unlock (lock);
-
return r;
}
diff --git a/stdlib/exit.c b/stdlib/exit.c
index c0b6d666c7..dfc4474891 100644
--- a/stdlib/exit.c
+++ b/stdlib/exit.c
@@ -19,11 +19,14 @@
#include <stdlib.h>
#include <unistd.h>
#include <sysdep.h>
+#include <libc-lock.h>
#include "exit.h"
#include "set-hooks.h"
DEFINE_HOOK (__libc_atexit, (void))
+/* initialise the processing complete flag to 0 */
+int __exit_funcs_done = 0;
/* Call all functions registered with `atexit' and `on_exit',
in the reverse of the order in which they were registered
@@ -44,14 +47,30 @@ __run_exit_handlers (int status, struct exit_function_list **listp,
the functions registered with `atexit' and `on_exit'. We call
everyone on the list and use the status value in the last
exit (). */
- while (*listp != NULL)
+ while (true)
{
- struct exit_function_list *cur = *listp;
+ struct exit_function_list *cur;
+
+ restart:
+ __libc_lock_lock (__exit_funcs_lock);
+ cur = *listp;
+
+ if (cur == NULL)
+ {
+ /* Exit processing complete. We will not allow any more
+ atexit/on_exit registrations. */
+ __exit_funcs_done = 1;
+ __libc_lock_unlock (__exit_funcs_lock);
+ break;
+ }
while (cur->idx > 0)
{
const struct exit_function *const f =
&cur->fns[--cur->idx];
+ const uint64_t new_exitfn_called = __new_exitfn_called;
+
+ __libc_lock_unlock (__exit_funcs_lock);
switch (f->flavor)
{
void (*atfct) (void);
@@ -83,6 +102,15 @@ __run_exit_handlers (int status, struct exit_function_list **listp,
cxafct (f->func.cxa.arg, status);
break;
}
+ __libc_lock_lock (__exit_funcs_lock);
+
+ if (__glibc_unlikely (new_exitfn_called != __new_exitfn_called))
+ {
+ /* The last exit function, or another thread, has registered
+ more exit functions. Start the loop over. */
+ __libc_lock_unlock (__exit_funcs_lock);
+ goto restart;
+ }
}
*listp = cur->next;
@@ -90,6 +118,8 @@ __run_exit_handlers (int status, struct exit_function_list **listp,
/* Don't free the last element in the chain, this is the statically
allocate element. */
free (cur);
+
+ __libc_lock_unlock (__exit_funcs_lock);
}
if (run_list_atexit)
diff --git a/stdlib/exit.h b/stdlib/exit.h
index 7f2e679246..023e81b83f 100644
--- a/stdlib/exit.h
+++ b/stdlib/exit.h
@@ -20,6 +20,7 @@
#include <stdbool.h>
#include <stdint.h>
+#include <libc-lock.h>
enum
{
@@ -62,6 +63,10 @@ extern struct exit_function_list *__quick_exit_funcs attribute_hidden;
extern struct exit_function *__new_exitfn (struct exit_function_list **listp);
extern uint64_t __new_exitfn_called attribute_hidden;
+__libc_lock_define (extern, __exit_funcs_lock);
+
+/* flag to mark that all registered atexit/onexit handlers are called */
+extern int __exit_funcs_done attribute_hidden;
extern void __run_exit_handlers (int status,
struct exit_function_list **listp,
diff --git a/stdlib/on_exit.c b/stdlib/on_exit.c
index 83845e76d8..2db3063db9 100644
--- a/stdlib/on_exit.c
+++ b/stdlib/on_exit.c
@@ -24,10 +24,16 @@
int
__on_exit (void (*func) (int status, void *arg), void *arg)
{
- struct exit_function *new = __new_exitfn (&__exit_funcs);
+ struct exit_function *new;
+
+ __libc_lock_lock (__exit_funcs_lock);
+ new = __new_exitfn (&__exit_funcs);
if (new == NULL)
- return -1;
+ {
+ __libc_lock_unlock (__exit_funcs_lock);
+ return -1;
+ }
#ifdef PTR_MANGLE
PTR_MANGLE (func);
@@ -36,6 +42,7 @@ __on_exit (void (*func) (int status, void *arg), void *arg)
new->func.on.arg = arg;
atomic_write_barrier ();
new->flavor = ef_on;
+ __libc_lock_unlock (__exit_funcs_lock);
return 0;
}
weak_alias (__on_exit, on_exit)
More information about the Libc-alpha
mailing list