Re: HAVE_GMTIME_R?

[Hallvard B Furuseth <h.b.furuseth@usit.uio.no>]

masarati@aero.polimi.it writes:
>> The gcc warning explained clearly enough what was wrong.
> 
> I meant: in terms of opportunity.  You can fix it directly by declaring
> the structure, or by anticipating the inclusion of lutil.h, since
> ldap_pvt.h depends on it.

Fine by me either way.  Users of the new function depend on lutil.h, the
rest of ldap_pvt.h does not.

>> I still disagree that moving gmtime_mutex out of slapd was the right
>> thing to do in the middle of RE24 though.  Though i suppose we could
>> '#define gmtime_mutex ldap_int_gmtime_mutex' so slapd at least remains
>> source-code compatible.
> 
> Mmmmh, ldap_int_gmtime_mutex is (and should in the long term remain)
> static.

Um, this is blowing up to a bit bigger issue than warranted, but anyway:

Long term, possibly - but only if we provide it to other packages
somehow or accept another mutex from them.  See below.  But in RE24, it
_was_ exported.

> I see your point in hiding a symbol within bugfix releases, but I
> wonder how many applications cared about that in the first place, and how
> many developers are aware of the issue.

Either we try to be upgrade-friendly or not.  Probably very few
third-party modules are affected by this particular issue, just like
very few are affected by many other particular issues.  But these
improbabilities do add up until we get an upgrade-unfriendly package.

That doesn't translate to an absolute freeze of e.g. slap.h between
major releases, but we can at least avoid changes that could just as
well be done in a compatible way.  Previously there was one correct
way to do it, now there is one different correct way, and - quite
unnecessarily - there is no overlap beween the two.

As for this particular mutex and the corresponding ctime mutex, all
threaded packages using gmtime and localtime must cooperate in order to
get correct behavior.  For example:

nm /usr/lib/lib<whatever>.a | egrep 'localtime|gmtime|ctime' | sort -u

/usr/lib/libsasl2.a:	ctime  localtime
/usr/lib/libssl.a:	gmtime localtime
/usr/lib/libcrypto.a	OPENSSL_gmtime X509_gmtime_adj gmtime_r localtime

I haven't looked too closely at the code, but possibly there is no way
for a program using OpenSSL or Cyrus SASL to use the non-_r functions
correctly.  Didn't find any mutex protecting them, anyway.

Come to think of it, this might explain some of the syncrepl timing
problems.

Anyway, if they all use the time functions, they must all use the same
mutex or the same wrapper function.

> In general, I realize the library design needs to take into account two
> different issues related to re-entrancy:
> 
> 1) the need for thread-safe behavior of libldap when usen in multithread
> clients (libldap_r)

Yup.  Which we really need to get around to officially supporting.  If
nothing else, because it gets silly to complain about other
non-reentrant third-party code.

> 2) the need to cure potential flaws in non re-entrant libraries OpenLDAP's
> depend on.

The general cure for (2) is to run slapd single-threaded.  Specific such
issues of re-entrancy will have to be handled on a case-by-case basis.

> (...)
> As an entirely separate issue, we need to provide means to make
> thread-safe the use of potentially unsafe functions that we need.  This is
> a bit harder, because:
> 
> a) we may need to use potentially unsafe functions in pieces of code not
> necessarily designed for thread-safety (e.g. liblutil in this case)
> 
> b) we may need to cooperate with other protections put in place by
> applications.

Yes.  And we need to _not_ insist that other packages do things Our Way.
3 packages that all insist that others do it their way cannot cooperate.

-- 
Hallvard