Re: commit: ldap/libraries/libldap_r rmutex.c threads.c

[Howard Chu <hyc@symas.com>]

Hallvard B Furuseth wrote:
> Howard Chu writes:
>   
> > This is just a quickie to get the functionality working.
> >     
>
> It doesn't though:-)
>
>   
Argh.
> Opheader.oh_tid (alias Operation.o_tid) is never set, yet
> overlays/accesslog.c uses the value as the owner thread ID.
>
> Did it exist once and get replaced with the 'ctx' things, or
> something like that?  Anyway, I suppose the variable can be
> moved from the Operation structure to accesslog's private data.
>   

Hm, I don't know what happened with oh_tid, but I think it would be 
better to actually set it, in case others need the thread ID in the 
future. On some systems pthread_self() is an expensive call, best to 
avoid calling it needlessly, especially since we have only a small pool 
of threads; we can get the thread IDs once and never need to ask again.
> A few notes on rmutex:
>
> - I don't see why you wrap the struct it in a pointer.  It does mean one
>   can move an rmutex in memory, but it replaces a mutex variable which
>   couldn't be moved moved.
>   

Just copying the approach from rdwr.c. In particular, this keeps the 
rmutex itself an opaque structure, which I think is desirable overall, 
without preventing its use in other structure declarations, etc.
> - In ldap_pvt_thread_rmutex_unlock(), since you do error checking here:
> 	ldap_pvt_thread_mutex_lock( &rm->ltrm_mutex );
> 	if( !ldap_pvt_thread_equal( owner, rm->ltrm_owner )) {
> 		ldap_pvt_thread_mutex_unlock( &rm->ltrm_mutex );
> 		return LDAP_PVT_THREAD_EINVAL;
> 	}
>   I'd also do replace the ldap_pvt_thread_mutex_lock() with
> 	rc = ldap_pvt_thread_mutex_lock( &rm->ltrm_mutex );
> 	if( rc )
> 		return rc;
>   and then maybe the other test can just as well be deleted, since
>   slapd doesn't check the return value anyway:-)  If so, the 'owner'
>   argument to ...unlock() can be dropped.
>   

Removing the test makes no sense, ltrm_mutex is not held for the 
duration of the rmutex.

--
 -- Howard Chu
 Chief Architect, Symas Corp.  http://www.symas.com
 Director, Highland Sun        http://highlandsun.com/hyc
 OpenLDAP Core Team            http://www.openldap.org/project/