Re: (ITS#5370) race condition in slap_op_time()

[hyc@symas.com]

h.b.furuseth@usit.uio.no wrote:
> Full_Name: Hallvard B Furuseth
> Version: HEAD, RE23, RE24
> OS:
> URL:
> Submission from: (NULL) (129.240.6.233)
> Submitted by: hallvard
>
>
> slapd/operation.c:slap_op_time() can decrease last_time:
>
> thread 1:   *t = slap_get_time();
> <clock:     increment time()>
> thread 2:   *t = slap_get_time();
> thread 2:   with mutex lock: increment last_time, set last_incr = 0;
> thread 1:   with mutex lock: decrement last_time, set last_incr = 0;
>
> The simplest fix is to move the slap_get_time() call inside the
> (global) slap_op_mutex lock.
>
> However as far as I can tell, only the accesslog overlay uses
> op->o_tincr, the value which needs the mutex.  And accesslog
> calls slap_op_time() itself when it needs that value.
> So maybe we should remove the op->o_tincr field.  Other calls
> to slap_op_time() can be replaced with slap_get_time().

Sounds like a good idea. It was somewhat redundant with the CSN counter 
(except that this was for all operations, and CSN was only for write 
operations) and it never became generally useful. (Shifted to microsecond 
timestamps in 2.4, because tincr isn't useful across multiple servers.) I 
guess it makes more sense to make it specific to accesslog.

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