This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH] Race in dl code: dl_open_worker and _dl_fini
- From: Sripathi Kodi <sripathik at in dot ibm dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Tue, 17 Apr 2007 14:49:14 +0530
- Subject: Re: [PATCH] Race in dl code: dl_open_worker and _dl_fini
- References: <20061222081928.GA11656@in.ibm.com> <20061225053426.GA8174@in.ibm.com> <20070413162605.GH1826@sunsite.mff.cuni.cz>
On Friday 13 April 2007 21:56, Jakub Jelinek wrote:
> On Mon, Dec 25, 2006 at 11:04:26AM +0530, Sripathi Kodi wrote:
> > I have attached a patch to this mail that implements the second solution
> > I have explained below. I have added a flag l_fini_called to link_map
> > structure and I have used that flag in _dl_fini to ensure the
> > destructors are called only once.
> > Please let me know if either of these two approaches is acceptable.
> Using a bit-field for this has issues (but so did the old code).
> As that part of _dl_fini is executed without dl_load_lock, some other
> thread modifying any of the bit-fields in the same word might race
> with it. Similarly --l->l_direct_opencount is unsafe, wonder if
> we need that at all, the app is going to exit anyway.
In _dl_fini, we could delay unlocking dl_load_lock till the end of the
function, but it looks like other functions that modify these bit fields
don't hold dl_load_lock. Since nobody else has reported races while accessing
these bit fields, I wonder if it is okay to integrate my patch and leave the
rest of the code as it is :-) What do you think?
> The more important question is, what is supposed to happen when
> calling functions from a library whose destructors have been run already.
Yes, _dl_fini seems to run the destructors even as other threads are using the
library. However, I noticed that dl_close, in most cases, doesn't run the
destructor of the shared library it is closing. It just comes out at this
if (__builtin_expect (map->l_flags_1 & DF_1_NODELETE, 0)
/* Nope. Do nothing. */
We could put the same check in _dl_fini to prevent running the destructors if
the library is in use. But since the process is exiting, I guess it is less
harmful to run destructors in _dl_fini than running them in dl_close.
However, running the destructor could surely cause some problem for other
threads even before process exit happens. Do you have any suggestion about
how we should handle this?