[Date Prev][Date Next]
Re: Proposed patch to support: draft-zeilenga-ldap-c-api-concurrency
> I will take a look at reworking the
> On 08/22/10 10:09 AM, firstname.lastname@example.org wrote:
>>> My concern is: can you guarantee that the occurrences of
>>> those additional mutexes, combined to the existing ones, do not result
>>> deadlocks? I mean: did you explicitly check all possible logical paths
>>> so, or take measures to avoid this possibility?
>> Another mostly "style" comment: I think you shouldn't be polluting
>> libldap's space with thread implementation specific details, like
>> #if defined(HAVE_THR)
>> #elif defined(HAVE_PTHREADS)
>> #if !(defined(HAVE_THR) || defined(HAVE_PTHREADS))
>> and so. I understand PTHREAD_MUTEX_INITIALIZER is handy, but the whole
>> thing should remain confined in libldap_r specific files.
> I will take a look at reworking this for the next round of code review.
#define LDAP_PVT_MUTEX_INITIALIZER DEFAULTMUTEX
#define LDAP_PVT_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
#define LDAP_PVT_MUTEX_INITIALIZER NULL
then use LDAP_PVT_MUTEX_INITIALIZER in code; you could then add a
ldap_pvt_thread_mutex_init_if(mutex) that assumes mutex was initialized
with LDAP_PVT_MUTEX_INITIALIZER; for THR and PTHREADS this call should be
empty, while in the remaining cases it would call
ldap_pvt_thread_mutex_init() only if mutex is not equal to
Also, I get some nasty warnings from other parts of the code that
(admittedly incorrectly) include ldap-int.h because
LDAP_PVT_THREAD_ASSERT_MUTEX_OWNER() is being redefined. I suggest that
instead of #defining it if LDAP_R_COMPILE is not defined, you rather
#define LDAP_ASSERT_MUTEX_OWNER(mutex) \
LDAP_PVT_THREAD_ASSERT_MUTEX_OWNER( mutex )
so you avoid mixing LDAP_PVT_THREAD_* and LDAP_* macros.