This is the mail archive of the
libc-help@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix the race between atexit() and exit()
Carlos O'Donell wrote:
On Tue, Jun 10, 2008 at 8:25 AM, Anoop <acv@linux.vnet.ibm.com> wrote:
Please see https://www.opengroup.org/austin/mailarchives/ag/ for the
discussions that happened
on the opengroup list.
Interesting discussion.
The result of this discussion as I would infer is that exit() and atexit()
need to be safe with each other in the matter of keeping the list
consistent.
But it CAN'T be guaranteed that, if atexit() is called, 'during or after'
exit() is called,
from a different thread, the handler that atexit() tries to register will be
invoked by exit().
I agree.
The proposed patch does 2 things -
1. Keeps the list in a consistent state by protecting it with lock
2. Fails the atexit() registrations after exit() finishes processing the
list.
That sounds reasonable.
Accommodating as many handlers as possible is the important and right thing
to do. But you cant
avoid the "visibility problem" where a new list head appears only after
exit() processing is over.
This problem should be taken care of, more by an application programmer than
by glibc.
Making the registration fail may not be of any worth as the thread might not
get a chance to take
any action afterwards, but from glibc perspective this is just for the sake
of keeping with the standard.
Where's the patch?
Cheers,
Carlos.
I had it posted in libc-alpha - http://sources.redhat.com/ml/libc-alpha/2008-06/msg00002.html
Posting it here as well. (Same approach as cxa_finilize)
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 */