On Fri, Jan 23, 2009 at 11:14 AM, Joe Orton <jorton@redhat.com> 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 <covener@gmail.com> wrote:
> > On Fri, Jan 23, 2009 at 3:46 AM, Graham Leggett <minfrin@sharp.fm> 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_*

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:

# undef HAVE_CTIME_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.

        *buf = NULL;
        *result = gethostbyname( name );
#error FOO        <- angry compiler

        if (*result!=NULL) {
                return 0;

        *herrno_ptr = h_errno;

        return -1;

This is Solaris, where gethostbyname() simply cannot be used from multiple threads.