Re: nss_ldap broken

On Mon, 2004-03-29 at 18:32, Jacques A. Vidrine wrote:
> On Fri, Mar 26, 2004 at 05:51:02PM -0500, Daniel Eischen wrote:
> > I think I made a comment about how you should always
> > prefix _pthread_foo() calls with 'if (__isthreaded)'.
> 
> Yes, I'm sure you did.  My recollection was that it was an
> optimization only, but it seems either I misunderstood or my
> recollection is poor (or both) :-)
> 
> > When the thread libraries are initialized, then overrwrite
> > the function pointers in libc's thread jumptable.  If you
> > unload the library, libc still retains those pointers.
> 
> OK, so we guard calls to threading routines with __isthreaded.  (Patch
> below.)  Uglifies things a bit, but I can deal.  Maybe some day we'll
> rewrite reentrant.h so that it doesn't lose the return code (they
> should all be like mutex_trylock?).
> 
> 
> So, if I understand correctly:
> 
>  (1)  __isthreaded starts out 0
>  (2)  When a threading library is loaded (by any cause? DT_NEEDED?
>       dlopen RTLD_GLOBAL? dlopen RTLD_LOCAL?), __isthreaded is set
>       to 1
>  (3)  When a threading library is unloaded, __isthreaded is reset to 0
> 
> Only, I don't immediately see where (3) happens...
> 
> Sean, could you report how this patch works for you?  Hmm, actually, it
> looks almost identical to what you posted :-)  Is there a reason that
> you stored the value of `__isthreaded' in a local variable?  Did that
> make a difference for your case?
> 
> Cheers,

My reasoning behind storing in a local variable was this:

1) __isthreaded starts out 0 in those routines.
2) lock is called for unthreaded model (i.e. noop?)
3) module with pthreads gets loaded.
4) unlock is called for threaded model (i.e. real unlock of bogus lock)

So I thought that if you didn't actually lock it threaded, then you
shouldn't unlock it threaded.

The most simple solution, I guess, is to just not unlock it in the
nss_atexit.  Since the program is going away it isn't really necessary.

I think the more correct solution is the one I proposed.

Cheers,
Sean