[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20150724212522.GF16376@brightrain.aerifal.cx>
Date: Fri, 24 Jul 2015 17:25:22 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] fix atexit when it is called from an atexit
handler
On Thu, Jul 23, 2015 at 05:07:21AM +0200, Szabolcs Nagy wrote:
> * Rich Felker <dalias@...c.org> [2015-07-22 21:18:16 -0400]:
> > On Thu, Jul 23, 2015 at 01:44:06AM +0200, Szabolcs Nagy wrote:
> > > void __funcs_on_exit()
> > > {
> > > int i;
> > > void (*func)(void *), *arg;
> > > LOCK(lock);
> > > - for (; head; head=head->next) for (i=COUNT-1; i>=0; i--) {
> > > - if (!head->f[i]) continue;
> > > + for (;;) {
> > > + i = next();
> > > + if (i<0) break;
> > > func = head->f[i];
> > > arg = head->a[i];
> > > head->f[i] = 0;
> >
> > I agree that this change should be made, but I don't like the
> > particulars of the patch. It seems to increase exit handler runtime to
> > O(n²) even in the case where no atexit is happening during exit, and
> > it's not very elegant. I think a simpler solution would be to reset i
> > to COUNT after calling f[i] if either (1) head has changed, or (2)
> > i<COUNT-1 and f[i+1] is nonzero. Does this sound correct/better?
>
> (2) should be 'f[i] is nonzero'.
>
> i didnt think about detecting the change of head,
> but it is simpler and better for the common case.
> >From 63c2b2d58b2ec45df67927b8959ea8af6dd578ed Mon Sep 17 00:00:00 2001
> From: Szabolcs Nagy <nsz@...t70.net>
> Date: Wed, 22 Jul 2015 23:05:57 +0000
> Subject: [PATCH] fix atexit when it is called from an atexit handler
>
> The old code accepted atexit handlers after exit, but did not run
> them. C11 seems to explicitly allow atexit to fail in this case,
> but this situation can easily come up in C++ if a destructor has
> local static object with a destructor so it should be handled.
>
> Restart iterating the list of atexit handlers if new handlers are
> added during a call.
>
> Note that the memory usage can grow linearly with the overall
> number of registered atexit handlers instead of with the worst
> case list length. (This only matters if atexit handlers keep
> registering atexit handlers which should not happen in practice).
> ---
> src/exit/atexit.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/src/exit/atexit.c b/src/exit/atexit.c
> index be82718..75f7511 100644
> --- a/src/exit/atexit.c
> +++ b/src/exit/atexit.c
> @@ -17,6 +17,7 @@ static volatile int lock[2];
> void __funcs_on_exit()
> {
> int i;
> + struct fl *old;
> void (*func)(void *), *arg;
> LOCK(lock);
> for (; head; head=head->next) for (i=COUNT-1; i>=0; i--) {
> @@ -24,9 +25,12 @@ void __funcs_on_exit()
> func = head->f[i];
> arg = head->a[i];
> head->f[i] = 0;
> + old = head;
> UNLOCK(lock);
> func(arg);
> LOCK(lock);
> + if (head != old || head->f[i])
> + i = COUNT;
> }
> }
I've committed the alternate version we discussed on IRC that avoids
scanning over the array to find the current position by using an extra
int of global state, the 'next slot' in head, which is protected by
the existing lock. This code is not heavily tested, so testing would
be welcome.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
