On Fri, Jan 23, 2009 at 11:14 AM, Joe Orton <[email protected]> wrote:
> On Fri, Jan 23, 2009 at 07:10:36AM -0500, Jeff Trawick wrote: > > On Fri, Jan 23, 2009 at 7:00 AM, Eric Covener <[email protected]> wrote: > > > On Fri, Jan 23, 2009 at 3:46 AM, Graham Leggett <[email protected]> > wrote: > > > > This is the first time I have heard of ldap_r. Is there a reason why > we > > > > can't just try bind to ldap_r first, and if that fails fall back to > ldap? > > > > > > I think that's the way to go. > > > > Yeah, sounds right. > > > > FWIW, /usr/sbin/httpd.worker on RHEL 5 U2 loads just plain libldap (and > > libldap_r is a distinct, and larger, library). A naive observer (okay, > me) > > would have expected that to be an explosive combination that would have > been > > fixed long ago. > > From reading the OpenLDAP source code the libldap_r build appears to > compile in a bunch of mutex locking calls around many/most of the ldap_* > interfaces. > > It's not obvious to me whether: > > a) this is because those calls are in fact manipulating process-global > state in some thread-unsafe way (not obvious how, if so), or > > b) this is enabling an additional API guarantee, that concurrent use of > a single LDAP * object from multiple threads is safe. > (I had a reason to look at this in a little more detail; here are my notes.) AFAICT, both a) and b) are true Besides the more obvious issue b (above), the reentrant build i. uses thread-safe forms of C library functions where available ii. uses mutexes around calls to thread-unsafe library functions that can't be avoided and, perhaps suprisingly, the non-reentrant build purposefully forgets about thread-safe forms of C library functions. Example from util-int.c, which encapsulates a number of library functions with thread safety issues: #ifndef LDAP_R_COMPILE # undef HAVE_REENTRANT_FUNCTIONS # undef HAVE_CTIME_R # undef HAVE_GETHOSTBYNAME_R # undef HAVE_GETHOSTBYADDR_R So, apart from the unforgotten detail of which flavor of libldap is linked to by the code you want to combine: Applications with multiple threads sharing LDAP connections need to use libldap_r, for proper sharing of the connection. Slightly less obvious: Applications with multiple threads calling OpenLDAP need to use libldap_r, even if they don't share LDAP connections; otherwise, resolver and certain other calls made for independent LDAP connections will not be thread-safe and may clobber global data owned by the system library. Even less obvious are the implications for applications which use OpenLDAP only on a single thread but have other threads running arbitrary code: Generally, all code in a multi-threaded application should use threadsafe forms of all library functions (you're broken as soon as you get two callers to gethostbyname(), for example). libldap_r is the build that maximizes the use of threadsafe forms of library functions, so it needs to be used in this case. The mutexes used by libldap_r for callers of library functions with no threadsafe form (or no form on this platform) won't protect against non-OpenLDAP callers of the function (since they don't share that mutex), so it is by no means perfect, but hopefully the number of non-threadsafe library functions that must be used on modern systems is extremely small. > > I would assume any user of this apr-util API, hahaha sorry, let me start > again.... I would assume that httpd does not rely on the additional API > guarantee in (b). > > Since libldap and libldap_r seem to implement the same set of symbols > (and without symbol versioning) this kind of issue is a minefield for > downstream distributors, if apr-util/httpd link against libldap_r and > some perl LDAP foo links against libldap, everything goes boom. > Downstream distributors shouldn't have any problem figuring this stuff out, and they have at least some control of what libldap vs. libldap_r mean anyway. httpd's LDAP support has to use libldap_r instead of libldap within a threaded MPM*, and other code which can be combined with it needs to accommodate :( *obviously it may not be practical to restrict the use of libldap_r to this minimal extent --/-- To sanity check that libldap really disables the reentrant forms of library functions, I found the call to plain gethostbyname() with no protection of the hostent structure in util-int.c and confirmed that it is indeed used when building libldap. #else *buf = NULL; *result = gethostbyname( name ); #error FOO <- angry compiler if (*result!=NULL) { return 0; } *herrno_ptr = h_errno; return -1; #endif This is Solaris, where gethostbyname() simply cannot be used from multiple threads.
