Re: nss_ldap broken

On Mon, 2004-03-29 at 19:43, Jacques A. Vidrine wrote:
> On Mon, Mar 29, 2004 at 10:31:56PM -0500, Daniel Eischen wrote:
> > On Mon, 29 Mar 2004, 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) :-)
> > 
> > I had no idea that libpthread would be loaded then unloaded.
> 
> Me neither :-)
> 
> > >  (2)  When a threading library is loaded (by any cause? DT_NEEDED?
> > >       dlopen RTLD_GLOBAL? dlopen RTLD_LOCAL?), __isthreaded is set
> > >       to 1
> > 
> > No, __isthreaded is only set to 1 (non-zero) when the first
> > thread (other than main) is created.  But the library is
> > auto-initialized and that's when libc's jump table is
> > filled.
> 
> Gotcha, thanks for the clarification.
> 
> > >  (3)  When a threading library is unloaded, __isthreaded is reset to 0
> > 
> > No, once threaded always threaded.  There's really no going
> > back.
> 
> So e.g. dlopen(libpthread) ... dlclose(libpthread) is not supported?
> 
> And following from that, dlopen(object_which_depends_on_libpthread) ...
> dlclose(object_which_depends_on_libpthread) may also not be supported?
> 
> [...]
> > I'm unsure how nss_ldap was built to depend on libpthread (or
> > any threads library).  I built it from ports and 'ldd' didn't
> > report any dependency on a threads library.
> 
> Interesting.  Rebuilt a few days ago after updating to OpenLDAP 2.1:
> 
> % ldd /usr/local/lib/nss_ldap.so.1
> /usr/local/lib/nss_ldap.so.1:
>         libldap.so.2 => /usr/local/lib/libldap.so.2 (0x2815f000)
>         liblber.so.2 => /usr/local/lib/liblber.so.2 (0x28189000)
>         libpthread.so.1 => /usr/lib/libpthread.so.1 (0x28194000)
>         libssl.so.3 => /usr/lib/libssl.so.3 (0x281b7000)
>         libcrypto.so.3 => /lib/libcrypto.so.3 (0x281e4000)
> 

Not only that, but libldap.so.2:
        liblber.so.2 => /usr/local/lib/liblber.so.2 (0x2818b000)
        libpthread.so.1 => /usr/lib/libpthread.so.1 (0x28197000)
        libsasl2.so.2 => /usr/local/lib/libsasl2.so.2 (0x281bb000)
        libssl.so.3 => /usr/lib/libssl.so.3 (0x281d0000)
        libcrypto.so.3 => /lib/libcrypto.so.3 (0x28201000)
        libc.so.5 => /lib/libc.so.5 (0x28080000)

So it seems unlikely that we can make nss_ldap.so not pull in pthread.

> 
> > An example of how to build a library that is thread-safe
> > but doesn't bring in the threads library is libgcc.  It uses
> > weak references (not definitions, like you see in our libc
> > and libpthread) to the necessary locking functions and
> > pthread_create.  For instance:
> > 
> > 	#pragma weak	pthread_create
> > 	#pragma weak	pthread_mutex_lock
> > 	#pragma weak	pthread_mutex_unlock
> > 	...
> > 
> > 	static void
> > 	foo(void)
> > 	{
> > 		...
> > 		if (pthread_create != NULL) {
> > 			pthread_mutex_lock(&foo_lock);
> > 			...
> > 			pthread_mutex_unlock(&foo_lock);
> > 		}
> > 		...
> > 	}
> > 
> > By making the pthread_* references weak, you don't need to be linked
> > to the threads library; they will be NULL if it is not present.
> > But if an application is linked to the threads library, then
> > those references won't be NULL.  There may be a little more
> > fu to it, but that's the general idea.
> 
> Huh.  Can/Should something like this be hidden within <pthread.h>, I
> wonder?
> 

There are many programs and libraries out there that are pulling in
pthread just because they want to be thread-safe (just a guess).  Of
course there are also those libraries that actually use threads, but it
is admirable to make any library thread-safe even if no threads are
used.

Back to the problem at hand, I still think the fix presented by Jacques
will work thus far.  If it is a threaded program, then pthread will not
be unloaded by the module (ref count). If we had an nss_*.so.1 module
that actually started up a thread, then we'd still be in trouble,
however, as __isthreaded would be set to 1 and not reset to 0 when
pthread is unloaded.

In that case, perhaps just not doing the unlock in nss_atexit makes
sense.  As mentioned before, the program is going away so there is no
harm in keeping it.

Sean