[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 23 Jul 2015 23:51:53 +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, 15:58 -0400 schrieb Rich Felker:
> On Thu, Jul 23, 2015 at 09:19:13AM +0200, Jens Gustedt wrote:
> > Am Donnerstag, den 23.07.2015, 07:54 +0200 schrieb Jens Gustedt:
> > > I'd think this could be done even simpler by running the handlers in
> > > batches. ...
> >
> > I checked, the strict ordering constraints for the handlers don't
> > allow for such a strategy, so forget it.
>
> Indeed, this is not possible.
>
> > Looking at the code, I am a bit uncomfortable with the idea of a
> > potentially unlimited number of calloc calls during exit. I think it
>
> Is there a specific reason? This will only happen in pathological code
> that keeps registering atexit handlers, and eventually the allocations
> will fail and therefore atexit will fail, which it's allowed to do.
> The only thing pathological that happens is that memory usage keeps
> increasing even if you've balanced the running and registering of
> atexit handlers so that the total number registered always stays
> bounded, but we're still talking about extreme abuse of the atexit
> API, and I think optimizing this case without an argument that it's
> worthwhile would probably be premature optimization.
It is probably not worth for optimization, I agree. It is more a
question of safety. Have code fail early that is trapped in an
infinite loop of adding atexit handlers during exit.
> > would be good if we could restrict the maximal number of successful
> > calls to atexit during exit to 32. A calloc-free strategy could be to
> > save head to a tmp a the beginning of processing and to provide a
> > `struct fl` table on the stack of __funcs_on_exit.
>
> I'm not sure how this would be better. It would be more predictable,
> but could also probably break some excessive but "valid" use (like
> a huge chain of ctors getting called from an atexit handler and all
> registering dtors).
I think this is really excessive and probably very poor design. atexit
should not be abused to make an entry into the list on a per-object
base.
No application should expect to be able to submit more than 32
handlers. After that atexit is allowed to fail, so everything more
than that is not portable. I don't advocate to keep strictly on the 32
bound (as we do for at_quick_exit), but once the process has entered
the exit procedure, there should be pressure to get things terminated.
> > > 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.
> >
> > Since this is fairly simple, I just did it. Please find a new version
> > attached. (too much changes, a patch makes no sense.) This compiles,
> > but I have no code to test this.
>
> The current program in musl is not to treat replacements of locks by
> an atomics as an improvement, but rather the opposite.
Ok, I wasn't aware of it.
For the code in question, I am not sure that it is more complicated
than the LOCK/UNLOCK/re-LOCK scheme of the current implementation.
Generally, it is perhaps worth thinking to omit the LOCK/UNLOCK in
the __funcs_... loops altogether. This is a context that is guaranteed
to be single threaded, I think.
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.
