[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1437630843.3270.1.camel@inria.fr>
Date: Thu, 23 Jul 2015 07:54:03 +0200
From: Jens Gustedt <jens.gustedt@...ia.fr>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] fix atexit when it is called from an atexit
handler
Am Donnerstag, den 23.07.2015, 05:07 +0200 schrieb Szabolcs Nagy:
> * 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.
I'd think this could be done even simpler by running the handlers in
batches. Something like
(a) take the lock and save the head of the current list in a tmp
variable
(b) release the lock
(c) iterate through the list that was saved in tmp
(d) if now head is non-zero, goto (a)
This is clearly linear in the number of handlers that are ever insert
to the list.
For the current implementation this would need a bit of change in
__cxa_atexit because 0 for head wouldn't mean that it hasn't been
used, yet.
A similar improvement could be done for at_quick_exit. There I think
it makes not much sense accepting new handlers during processing. So
the strategy could just be
(i) Take the lock and save count in a tmp
(ii) set count to 32
(iii) unlock
(iv) do the processing
But looking at it, I think that one can even be more improved. We
could just use count as atomic, and so we wouldn't need a lock.
I can prepare a patch for that, if you like.
Jens
--
:: INRIA Nancy Grand Est ::: Camus ::::::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536 ::
:: :::::::::::::::::::::: gsm France : +33 651400183 ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::
Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)
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.
