[PATCH] Fix the race between atexit() and exit()
Anoop
acv@linux.vnet.ibm.com
Tue Jun 3 12:19:00 GMT 2008
POSIX requires that at normal program termination, all functions
registered by the atexit() function shall be called. A race exits among
__cxa_atexit() and exit() on __exit_funcs can cause a successfully registered
handler not to be called. I have a test-case which reveals this bug, which can
be posted if required. The following patch solves this by extending the locking
used by __new_exitfn() to exit() and failing atexit() registrations once the
exit() code completed traversing the __exit_funcs list.
Feedbacks appreciated.
Cheers
- Anoop
Signed-off-by: Anoop Vijayan <anoop.vijayan@in.ibm.com>
diff -Naurp glibc-2.7.orig/stdlib/cxa_atexit.c glibc-2.7/stdlib/cxa_atexit.c
--- glibc-2.7.orig/stdlib/cxa_atexit.c 2006-07-26 12:54:45.000000000 +0530
+++ glibc-2.7/stdlib/cxa_atexit.c 2008-06-03 02:36:57.000000000 +0530
@@ -51,7 +51,7 @@ INTDEF(__cxa_atexit)
/* We change global data, so we need locking. */
-__libc_lock_define_initialized (static, lock)
+__libc_lock_define_initialized (, __exit_funcs_lock)
static struct exit_function_list initial;
@@ -66,7 +66,14 @@ __new_exitfn (void)
struct exit_function *r = NULL;
size_t i = 0;
- __libc_lock_lock (lock);
+ __libc_lock_lock (__exit_funcs_lock);
+ if (__exit_funcs_done == 1)
+ {
+ /* exit code finished processing all handlers
+ so fail this registration */
+ __libc_lock_unlock (__exit_funcs_lock);
+ return NULL;
+ }
for (l = __exit_funcs; l != NULL; p = l, l = l->next)
{
@@ -117,7 +124,7 @@ __new_exitfn (void)
++__new_exitfn_called;
}
- __libc_lock_unlock (lock);
+ __libc_lock_unlock (__exit_funcs_lock);
return r;
}
diff -Naurp glibc-2.7.orig/stdlib/exit.c glibc-2.7/stdlib/exit.c
--- glibc-2.7.orig/stdlib/exit.c 2005-12-18 23:01:14.000000000 +0530
+++ glibc-2.7/stdlib/exit.c 2008-06-03 04:19:52.000000000 +0530
@@ -18,13 +18,17 @@
#include <stdio.h>
#include <stdlib.h>
+#include <atomic.h>
#include <unistd.h>
#include <sysdep.h>
+#include <bits/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
@@ -36,53 +40,87 @@ exit (int status)
the functions registered with `atexit' and `on_exit'. We call
everyone on the list and use the status value in the last
exit (). */
- while (__exit_funcs != NULL)
+ while(1)
{
- struct exit_function_list *old;
-
- while (__exit_funcs->idx > 0)
- {
- const struct exit_function *const f =
- &__exit_funcs->fns[--__exit_funcs->idx];
- switch (f->flavor)
- {
- void (*atfct) (void);
- void (*onfct) (int status, void *arg);
- void (*cxafct) (void *arg, int status);
-
- case ef_free:
- case ef_us:
- break;
- case ef_on:
- onfct = f->func.on.fn;
+ struct exit_function_list *funcs;
+ struct exit_function *f;
+restart:
+ __libc_lock_lock (__exit_funcs_lock);
+ funcs = __exit_funcs;
+ if(funcs==NULL)
+ {
+ /* mark the processing complete
+ we wont allow to register more handlers */
+ __exit_funcs_done = 1;
+ __libc_lock_unlock (__exit_funcs_lock);
+ break;
+ }
+ f = &funcs->fns[funcs->idx - 1];
+ uint64_t check1 = __new_exitfn_called;
+ __libc_lock_unlock (__exit_funcs_lock);
+ for(; f >= &funcs->fns[0]; --f)
+ {
+ uint64_t check2 = __new_exitfn_called;
+ switch (f->flavor)
+ {
+ void (*atfct) (void);
+ void (*onfct) (int status, void *arg);
+ void (*cxafct) (void *arg, int status);
+ void *arg;
+
+ case ef_free:
+ break;
+ case ef_us:
+ goto restart;
+ case ef_on:
+ onfct = f->func.on.fn;
+ arg = f->func.on.arg;
+ while(atomic_compare_and_exchange_bool_acq (&f->flavor,
ef_free, ef_on));
#ifdef PTR_DEMANGLE
- PTR_DEMANGLE (onfct);
+ PTR_DEMANGLE (onfct);
#endif
- onfct (status, f->func.on.arg);
- break;
- case ef_at:
- atfct = f->func.at;
+ onfct (status, arg);
+ break;
+ case ef_at:
+ atfct = f->func.at;
+ while(atomic_compare_and_exchange_bool_acq (&f->flavor,
ef_free, ef_at));
#ifdef PTR_DEMANGLE
- PTR_DEMANGLE (atfct);
+ PTR_DEMANGLE (atfct);
#endif
- atfct ();
- break;
- case ef_cxa:
- cxafct = f->func.cxa.fn;
+ atfct ();
+ break;
+ case ef_cxa:
+ cxafct = f->func.cxa.fn;
+ arg = f->func.cxa.arg;
+ while(atomic_compare_and_exchange_bool_acq (&f->flavor,
ef_free, ef_cxa));
#ifdef PTR_DEMANGLE
- PTR_DEMANGLE (cxafct);
+ PTR_DEMANGLE (cxafct);
#endif
- cxafct (f->func.cxa.arg, status);
- break;
- }
- }
-
- old = __exit_funcs;
- __exit_funcs = __exit_funcs->next;
- if (__exit_funcs != NULL)
- /* Don't free the last element in the chain, this is the statically
- allocate element. */
- free (old);
+ cxafct (arg, status);
+ break;
+ }
+
+ /* It is possible that that last exit function registered
+ more exit functions. Start the loop over. */
+ if (__builtin_expect (check2 != __new_exitfn_called, 0))
+ goto restart;
+ }
+
+ __libc_lock_lock (__exit_funcs_lock);
+ if (__builtin_expect (check1 != __new_exitfn_called, 0))
+ {
+ __libc_lock_unlock (__exit_funcs_lock);
+ goto restart;
+ }
+ else
+ {
+ __exit_funcs = __exit_funcs->next;
+ /* Don't free the last element in the chain, this is the statically
+ allocate element. */
+ if(__exit_funcs != NULL)
+ free(funcs);
+ __libc_lock_unlock (__exit_funcs_lock);
+ }
}
RUN_HOOK (__libc_atexit, ());
diff -Naurp glibc-2.7.orig/stdlib/exit.h glibc-2.7/stdlib/exit.h
--- glibc-2.7.orig/stdlib/exit.h 2006-07-26 12:53:43.000000000 +0530
+++ glibc-2.7/stdlib/exit.h 2008-06-03 02:36:57.000000000 +0530
@@ -21,7 +21,7 @@
#define _EXIT_H 1
#include <stdint.h>
-
+#include <bits/libc-lock.h>
enum
{
ef_free, /* `ef_free' MUST be zero! */
@@ -62,5 +62,9 @@ extern struct exit_function_list *__exit
extern struct exit_function *__new_exitfn (void);
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;
#endif /* exit.h */
More information about the Libc-alpha
mailing list