Re: nss_ldap broken

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,
-- 
Jacques Vidrine / nectar_at_celabo.org / jvidrine_at_verio.net / nectar_at_freebsd.org

Index: net/nsdispatch.c
===================================================================
RCS file: /home/ncvs/src/lib/libc/net/nsdispatch.c,v
retrieving revision 1.10
diff -c -r1.10 nsdispatch.c
*** net/nsdispatch.c	15 Mar 2004 08:14:35 -0000	1.10
--- net/nsdispatch.c	30 Mar 2004 02:19:19 -0000
***************
*** 319,324 ****
--- 319,325 ----
  	int		 result;
  	const char	*path;
  
+ 	result = 0;
  #if defined(_NSS_DEBUG) && defined(_NSS_SHOOT_FOOT)
  	/* NOTE WELL:  THIS IS A SECURITY HOLE. This must only be built
  	 * for debugging purposes and MUST NEVER be used in production.
***************
*** 331,346 ****
  		return (0);
  	if (statbuf.st_mtime <= confmod)
  		return (0);
! 	result = _pthread_mutex_trylock(&conf_lock);
! 	if (result != 0)
! 		return (0);
! 	(void)_pthread_rwlock_unlock(&nss_lock);
! 	result = _pthread_rwlock_wrlock(&nss_lock);
! 	if (result != 0)
! 		goto fin2;
  	_nsyyin = fopen(path, "r");
! 	if (_nsyyin == NULL)
  		goto fin;
  	VECTOR_FREE(_nsmap, &_nsmapsize, sizeof(*_nsmap),
  	    (vector_free_elem)ns_dbt_free);
  	VECTOR_FREE(_nsmod, &_nsmodsize, sizeof(*_nsmod),
--- 332,351 ----
  		return (0);
  	if (statbuf.st_mtime <= confmod)
  		return (0);
! 	if (__isthreaded) {
! 	    result = _pthread_mutex_trylock(&conf_lock);
! 	    if (result != 0)
! 		    return (0);
! 	    (void)_pthread_rwlock_unlock(&nss_lock);
! 	    result = _pthread_rwlock_wrlock(&nss_lock);
! 	    if (result != 0)
! 		    goto fin2;
! 	}
  	_nsyyin = fopen(path, "r");
! 	if (_nsyyin == NULL) {
! 		result = errno;
  		goto fin;
+ 	}
  	VECTOR_FREE(_nsmap, &_nsmapsize, sizeof(*_nsmap),
  	    (vector_free_elem)ns_dbt_free);
  	VECTOR_FREE(_nsmod, &_nsmodsize, sizeof(*_nsmod),
***************
*** 353,362 ****
  		(void)atexit(nss_atexit);
  	confmod = statbuf.st_mtime;
  fin:
! 	(void)_pthread_rwlock_unlock(&nss_lock);
! 	result = _pthread_rwlock_rdlock(&nss_lock);
  fin2:
! 	(void)_pthread_mutex_unlock(&conf_lock);
  	return (result);
  }
  
--- 358,371 ----
  		(void)atexit(nss_atexit);
  	confmod = statbuf.st_mtime;
  fin:
! 	if (__isthreaded) {
! 	    (void)_pthread_rwlock_unlock(&nss_lock);
! 	    if (result == 0)
! 		    result = _pthread_rwlock_rdlock(&nss_lock);
! 	}
  fin2:
! 	if (__isthreaded)
! 		(void)_pthread_mutex_unlock(&conf_lock);
  	return (result);
  }
  
***************
*** 510,521 ****
  static void
  nss_atexit(void)
  {
! 	(void)_pthread_rwlock_wrlock(&nss_lock);
  	VECTOR_FREE(_nsmap, &_nsmapsize, sizeof(*_nsmap),
  	    (vector_free_elem)ns_dbt_free);
  	VECTOR_FREE(_nsmod, &_nsmodsize, sizeof(*_nsmod),
  	    (vector_free_elem)ns_mod_free);
! 	(void)_pthread_rwlock_unlock(&nss_lock);
  }
  
  
--- 519,532 ----
  static void
  nss_atexit(void)
  {
! 	if (__isthreaded)
! 		(void)_pthread_rwlock_wrlock(&nss_lock);
  	VECTOR_FREE(_nsmap, &_nsmapsize, sizeof(*_nsmap),
  	    (vector_free_elem)ns_dbt_free);
  	VECTOR_FREE(_nsmod, &_nsmodsize, sizeof(*_nsmod),
  	    (vector_free_elem)ns_mod_free);
! 	if (__isthreaded)
! 		(void)_pthread_rwlock_unlock(&nss_lock);
  }
  
  
***************
*** 571,580 ****
  	int		 serrno, i, result, srclistsize;
  
  	serrno = errno;
! 	result = _pthread_rwlock_rdlock(&nss_lock);
! 	if (result != 0) {
! 		result = NS_UNAVAIL;
! 		goto fin;
  	}
  	result = nss_configure();
  	if (result != 0) {
--- 582,593 ----
  	int		 serrno, i, result, srclistsize;
  
  	serrno = errno;
! 	if (__isthreaded) {
! 		result = _pthread_rwlock_rdlock(&nss_lock);
! 		if (result != 0) {
! 			result = NS_UNAVAIL;
! 			goto fin;
! 		}
  	}
  	result = nss_configure();
  	if (result != 0) {
***************
*** 604,610 ****
  				break;
  		}
  	}
! 	(void)_pthread_rwlock_unlock(&nss_lock);
  fin:
  	errno = serrno;
  	return (result);
--- 617,624 ----
  				break;
  		}
  	}
! 	if (__isthreaded)
! 		(void)_pthread_rwlock_unlock(&nss_lock);
  fin:
  	errno = serrno;
  	return (result);
Index: net/nss_compat.c
===================================================================
RCS file: /home/ncvs/src/lib/libc/net/nss_compat.c,v
retrieving revision 1.2
diff -c -r1.2 nss_compat.c
*** net/nss_compat.c	9 Jan 2004 13:43:49 -0000	1.2
--- net/nss_compat.c	30 Mar 2004 02:16:03 -0000
***************
*** 41,46 ****
--- 41,47 ----
  #include <pthread.h>
  #include <pthread_np.h>
  #include "un-namespace.h"
+ #include "libc_private.h"
  
  
  struct group;
***************
*** 60,66 ****
  
  #define SET_TERMINATOR(x, y)						\
  do {									\
! 	if (_pthread_main_np())						\
  		_term_main_##x = (y);					\
  	else {								\
  		(void)_pthread_once(&_term_once_##x, _term_create_##x);	\
--- 61,67 ----
  
  #define SET_TERMINATOR(x, y)						\
  do {									\
! 	if (!__isthreaded || _pthread_main_np())			\
  		_term_main_##x = (y);					\
  	else {								\
  		(void)_pthread_once(&_term_once_##x, _term_create_##x);	\
***************
*** 69,75 ****
  } while (0)
  
  #define CHECK_TERMINATOR(x)					\
! (_pthread_main_np() ?						\
      (_term_main_##x) :						\
      ((void)_pthread_once(&_term_once_##x, _term_create_##x),	\
      _pthread_getspecific(_term_key_##x)))
--- 70,76 ----
  } while (0)
  
  #define CHECK_TERMINATOR(x)					\
! (!__isthreaded || _pthread_main_np() ?				\
      (_term_main_##x) :						\
      ((void)_pthread_once(&_term_once_##x, _term_create_##x),	\
      _pthread_getspecific(_term_key_##x)))