From f7e301004c55dbac3fb046697c5274c8eddafc14 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Wed, 20 Nov 2013 09:08:50 -0500
Subject: [PATCH] Ticket 47599 - Reduce lock scope in retro changelog plug-in

Description:  Use RW locks for protecting the change numbers.

              We still need to do the locking in retrocl_po.c as we need to
              serialize the actual updates.

https://fedorahosted.org/389/ticket/47599

Reviewed by: ?
---
 ldap/servers/plugins/retrocl/retrocl.c    |    3 +-
 ldap/servers/plugins/retrocl/retrocl.h    |    1 +
 ldap/servers/plugins/retrocl/retrocl_cn.c |   42 ++++++++++++++++++++---------
 ldap/servers/plugins/retrocl/retrocl_po.c |    2 +-
 4 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/ldap/servers/plugins/retrocl/retrocl.c b/ldap/servers/plugins/retrocl/retrocl.c
index 141cfd2..6148c02 100644
--- a/ldap/servers/plugins/retrocl/retrocl.c
+++ b/ldap/servers/plugins/retrocl/retrocl.c
@@ -481,7 +481,8 @@ retrocl_plugin_init(Slapi_PBlock *pb)
 	  if (!is_betxn) {
 	    rc= slapi_register_plugin_ext("internalpostoperation", 1 /* Enabled */, "retrocl_internalpostop_init", retrocl_internalpostop_init, "Retrocl internal postoperation plugin", NULL, identity, precedence);
 	  }
-
+	  retrocl_cn_lock = slapi_new_rwlock();
+	  if(retrocl_cn_lock == NULL) return -1;
 	  retrocl_internal_lock = PR_NewLock();
 	  if (retrocl_internal_lock == NULL) return -1;
 	}
diff --git a/ldap/servers/plugins/retrocl/retrocl.h b/ldap/servers/plugins/retrocl/retrocl.h
index 214f3af..3f808e6 100644
--- a/ldap/servers/plugins/retrocl/retrocl.h
+++ b/ldap/servers/plugins/retrocl/retrocl.h
@@ -131,6 +131,7 @@ extern const char *attr_nsuniqueid;
 extern const char *attr_isreplicated;
 
 extern PRLock *retrocl_internal_lock;
+extern Slapi_RWLock *retrocl_cn_lock;
 
 /* Functions */
 
diff --git a/ldap/servers/plugins/retrocl/retrocl_cn.c b/ldap/servers/plugins/retrocl/retrocl_cn.c
index d2b15a4..f816730 100644
--- a/ldap/servers/plugins/retrocl/retrocl_cn.c
+++ b/ldap/servers/plugins/retrocl/retrocl_cn.c
@@ -163,8 +163,9 @@ int retrocl_get_changenumbers(void)
 		       NULL,NULL,0,&cr,NULL,handle_cnum_result,
 		       handle_cnum_entry, NULL);
 
-    retrocl_first_cn = cr.cr_cnum;
+    slapi_rwlock_wrlock(retrocl_cn_lock);
 
+    retrocl_first_cn = cr.cr_cnum;
     slapi_ch_free(( void **) &cr.cr_time );
 
     slapi_seq_callback(RETROCL_CHANGELOG_DN,SLAPI_SEQ_LAST,
@@ -178,6 +179,8 @@ int retrocl_get_changenumbers(void)
 		    retrocl_first_cn,
 		    retrocl_internal_cn);
 
+    slapi_rwlock_unlock(retrocl_cn_lock);
+
     slapi_ch_free(( void **) &cr.cr_time );
 
     return 0;
@@ -238,10 +241,10 @@ time_t retrocl_getchangetime( int type, int *err )
 
 void retrocl_forget_changenumbers(void) 
 { 
-    PR_Lock(retrocl_internal_lock);
+    slapi_rwlock_wrlock(retrocl_cn_lock);
     retrocl_first_cn = 0;
     retrocl_internal_cn = 0;
-    PR_Unlock(retrocl_internal_lock);
+    slapi_rwlock_unlock(retrocl_cn_lock);
 }
 
 /*
@@ -258,9 +261,11 @@ void retrocl_forget_changenumbers(void)
 changeNumber retrocl_get_first_changenumber(void) 
 { 
     changeNumber cn;
-    PR_Lock(retrocl_internal_lock);
+
+    slapi_rwlock_rdlock(retrocl_cn_lock);
     cn = retrocl_first_cn;
-    PR_Unlock(retrocl_internal_lock);
+    slapi_rwlock_unlock(retrocl_cn_lock);
+
     return cn;
 }
 
@@ -277,9 +282,9 @@ changeNumber retrocl_get_first_changenumber(void)
 
 void retrocl_set_first_changenumber(changeNumber cn) 
 { 
-    PR_Lock(retrocl_internal_lock);
+    slapi_rwlock_wrlock(retrocl_cn_lock);
     retrocl_first_cn = cn;
-    PR_Unlock(retrocl_internal_lock);
+    slapi_rwlock_unlock(retrocl_cn_lock);
 }
 
 
@@ -297,9 +302,11 @@ void retrocl_set_first_changenumber(changeNumber cn)
 changeNumber retrocl_get_last_changenumber(void) 
 { 
     changeNumber cn;
-    PR_Lock(retrocl_internal_lock);
+
+    slapi_rwlock_rdlock(retrocl_cn_lock);
     cn = retrocl_internal_cn;
-    PR_Unlock(retrocl_internal_lock);
+    slapi_rwlock_unlock(retrocl_cn_lock);
+
     return cn;
 }
 
@@ -316,9 +323,11 @@ changeNumber retrocl_get_last_changenumber(void)
 
 void retrocl_commit_changenumber(void) 
 { 
+    slapi_rwlock_wrlock(retrocl_cn_lock);
     if ( retrocl_first_cn == 0) {
         retrocl_first_cn = retrocl_internal_cn;
     }
+    slapi_rwlock_unlock(retrocl_cn_lock);
 }
 
 /*
@@ -333,8 +342,10 @@ void retrocl_commit_changenumber(void)
  */
 
 void retrocl_release_changenumber(void) 
-{ 
+{
+    slapi_rwlock_wrlock(retrocl_cn_lock);
     retrocl_internal_cn--;
+    slapi_rwlock_unlock(retrocl_cn_lock);
 }
 
 /*
@@ -342,7 +353,7 @@ void retrocl_release_changenumber(void)
  *
  * Returns: 0/-1
  *
- * Arguments: none
+ * Arguments: none.  The caller should have taken write lock for the change numbers
  *
  * Description: reads the last entry in the changelog to obtain
  * the last change number.
@@ -355,6 +366,7 @@ int retrocl_update_lastchangenumber(void)
 
     if (retrocl_be_changelog == NULL) return -1;
 
+    slapi_rwlock_unlock(retrocl_cn_lock);
     cr.cr_cnum = 0;
     cr.cr_time = 0;
     slapi_seq_callback(RETROCL_CHANGELOG_DN,SLAPI_SEQ_LAST,
@@ -362,7 +374,7 @@ int retrocl_update_lastchangenumber(void)
                NULL,NULL,0,&cr,NULL,handle_cnum_result,
                handle_cnum_entry, NULL);
 
-
+    slapi_rwlock_wrlock(retrocl_cn_lock);
     retrocl_internal_cn = cr.cr_cnum;
     slapi_log_error(SLAPI_LOG_PLUGIN,"retrocl","Refetched last changenumber =  %lu \n",
             retrocl_internal_cn);
@@ -394,6 +406,8 @@ changeNumber retrocl_assign_changenumber(void)
      * validity of the internal assignment of retrocl_internal_cn 
      * we had from the startup */  
 
+    slapi_rwlock_wrlock(retrocl_cn_lock);
+
     if(retrocl_internal_cn <= retrocl_first_cn){ 
         /* the numbers have become out of sync - retrocl_get_changenumbers
          * gets called only once during startup and it may have had a problem 
@@ -404,8 +418,10 @@ changeNumber retrocl_assign_changenumber(void)
          */                                                         
         retrocl_update_lastchangenumber();                                   
     }                             
-
     retrocl_internal_cn++;
     cn = retrocl_internal_cn;
+
+    slapi_rwlock_unlock(retrocl_cn_lock);
+
     return cn;
 }
diff --git a/ldap/servers/plugins/retrocl/retrocl_po.c b/ldap/servers/plugins/retrocl/retrocl_po.c
index 9c005b3..b3bfafe 100644
--- a/ldap/servers/plugins/retrocl/retrocl_po.c
+++ b/ldap/servers/plugins/retrocl/retrocl_po.c
@@ -379,7 +379,7 @@ write_replog_db(
 	    retrocl_release_changenumber();
 	} else {
 	/* Tell the change numbering system this one's committed to disk  */
-	    retrocl_commit_changenumber( );
+	    retrocl_commit_changenumber();
 	}
     } else {
 	slapi_log_error( SLAPI_LOG_FATAL, RETROCL_PLUGIN_NAME, 
-- 
1.7.1

