Re: (ITS#7968) SIGSEGV shortly after reconnection performed by syncrepl due to synchronization conflicts

[leo@yuriev.ru]

This is a multi-part message in MIME format.
--------------040002050003040401050402
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

Partially fixed.
Patch is for current OPENLDAP_REL_ENG_2_4, but applicable for master.

Leonid.

--

The attached files is derived from OpenLDAP Software. All of the 
modifications
to OpenLDAP Software represented in the following patch(es) were 
developed by
Peter-Service LLC, Moscow, Russia. Peter-Service LLC has not assigned rights
and/or interest in this work to any party. I, Leonid Yuriev am authorized by
Peter-Service LLC, my employer, to release this work under the following
terms.

Peter-Service LLC hereby places the following modifications to OpenLDAP 
Software
(and only these modifications) into the public domain. Hence, these
modifications may be freely used and/or redistributed for any purpose 
with or
without attribution and/or other notice.




--------------040002050003040401050402
Content-Type: text/x-patch;
 name="slapd-syncrepl-locking.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="slapd-syncrepl-locking.patch"

commit e845cfba8b8c17d291fb810b762e2d2bdb22b016
Author: Leo Yuriev <leo@yuriev.ru>
Date:   2014-12-01 19:26:08 +0300

    ITS#7968: add locking into syncrepl.
    
    Mutex must be held even for a read-access of a cookie state,
    as there may be a race with write in other thread.

diff --git a/servers/slapd/syncrepl.c b/servers/slapd/syncrepl.c
index caab9f5..eb7bec2 100644
--- a/servers/slapd/syncrepl.c
+++ b/servers/slapd/syncrepl.c
@@ -926,6 +926,7 @@ do_syncrep2(
 					if ( syncCookie.ctxcsn ) {
 						int i, sid = slap_parse_csn_sid( syncCookie.ctxcsn );
 						check_syncprov( op, si );
+						ldap_pvt_thread_mutex_lock( &si->si_cookieState->cs_mutex );
 						for ( i =0; i<si->si_cookieState->cs_num; i++ ) {
 							/* new SID */
 							if ( sid < si->si_cookieState->cs_sids[i] )
@@ -935,15 +936,18 @@ do_syncrep2(
 									bdn.bv_val[bdn.bv_len] = '\0';
 									Debug( LDAP_DEBUG_SYNC, "do_syncrep2: %s CSN too old, ignoring %s (%s)\n",
 										si->si_ridtxt, syncCookie.ctxcsn->bv_val, bdn.bv_val );
+									si->si_too_old = 1;
+									ldap_pvt_thread_mutex_unlock( &si->si_cookieState->cs_mutex );
 									ldap_controls_free( rctrls );
 									rc = 0;
-									si->si_too_old = 1;
 									goto done;
 								}
 								si->si_too_old = 0;
 								break;
 							}
 						}
+						ldap_pvt_thread_mutex_unlock( &si->si_cookieState->cs_mutex );
+
 						/* check pending CSNs too */
 						while ( ldap_pvt_thread_mutex_trylock( &si->si_cookieState->cs_pmutex )) {
 							if ( slapd_shutdown ) {
@@ -953,6 +957,8 @@ do_syncrep2(
 							if ( !ldap_pvt_thread_pool_pausecheck( &connection_pool ))
 								ldap_pvt_thread_yield();
 						}
+
+						ldap_pvt_thread_mutex_lock( &si->si_cookieState->cs_mutex );
 						for ( i =0; i<si->si_cookieState->cs_pnum; i++ ) {
 							if ( sid < si->si_cookieState->cs_psids[i] )
 								break;
@@ -961,9 +967,10 @@ do_syncrep2(
 									bdn.bv_val[bdn.bv_len] = '\0';
 									Debug( LDAP_DEBUG_SYNC, "do_syncrep2: %s CSN pending, ignoring %s (%s)\n",
 										si->si_ridtxt, syncCookie.ctxcsn->bv_val, bdn.bv_val );
+									ldap_pvt_thread_mutex_unlock( &si->si_cookieState->cs_mutex );
+									ldap_pvt_thread_mutex_unlock( &si->si_cookieState->cs_pmutex );
 									ldap_controls_free( rctrls );
 									rc = 0;
-									ldap_pvt_thread_mutex_unlock( &si->si_cookieState->cs_pmutex );
 									goto done;
 								}
 								ber_bvreplace( &si->si_cookieState->cs_pvals[i],
@@ -978,6 +985,7 @@ do_syncrep2(
 								(struct sync_cookie *)&si->si_cookieState->cs_pvals,
 								i, sid, syncCookie.ctxcsn );
 						}
+						ldap_pvt_thread_mutex_unlock( &si->si_cookieState->cs_mutex );
 						assert( punlock < 0 );
 						punlock = i;
 					} else if (si->si_too_old) {
@@ -1032,6 +1040,7 @@ do_syncrep2(
 				/* on failure, revert pending CSN */
 				if ( rc != LDAP_SUCCESS ) {
 					int i;
+					ldap_pvt_thread_mutex_lock( &si->si_cookieState->cs_mutex );
 					for ( i = 0; i<si->si_cookieState->cs_num; i++ ) {
 						if ( si->si_cookieState->cs_sids[i] == si->si_cookieState->cs_psids[punlock] ) {
 							ber_bvreplace( &si->si_cookieState->cs_pvals[punlock],
@@ -1041,6 +1050,7 @@ do_syncrep2(
 					}
 					if ( i == si->si_cookieState->cs_num )
 						si->si_cookieState->cs_pvals[punlock].bv_val[0] = '\0';
+					ldap_pvt_thread_mutex_unlock( &si->si_cookieState->cs_mutex );
 				}
 				ldap_pvt_thread_mutex_unlock( &si->si_cookieState->cs_pmutex );
 			}


--------------040002050003040401050402--