[PATCH] Fix the race between atexit() and exit()
Anoop
acv@linux.vnet.ibm.com
Wed Jun 11 12:06:00 GMT 2008
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 */
More information about the Libc-help
mailing list