[389-commits] Branch 'Directory_Server_8_2_Branch' - ldap/servers

[Noriko Hosoi]

 ldap/servers/plugins/replication/repl5.h          |    2 -
 ldap/servers/plugins/replication/repl5_agmt.c     |    8 ++---
 ldap/servers/plugins/replication/repl5_protocol.c |   33 +++++++++++++++++++---
 3 files changed, 34 insertions(+), 9 deletions(-)

New commits:
commit bea4eb5bfa3d0f588628387fd43417982b47009c
Author: Noriko Hosoi <nhosoi at redhat.com>
Date:   Mon Feb 22 10:25:46 2010 -0800

    539618 - Replication bulk import reports Invalid read/write
    
    https://bugzilla.redhat.com/show_bug.cgi?id=539618
    
    Descriptions: When a protocol is freed by prot_free, prot_close
    is supposed to have been called to stop the main thread
    prot_thread_main.  But, there was no mechanism for the freeing
    thread whether the prot_thread_main has already quitted or not,
    it could have released the Repl_Protocol even though it was
    still being in use.  This fix is adding a checking method.

diff --git a/ldap/servers/plugins/replication/repl5.h b/ldap/servers/plugins/replication/repl5.h
index 97ce556..332bfb8 100644
--- a/ldap/servers/plugins/replication/repl5.h
+++ b/ldap/servers/plugins/replication/repl5.h
@@ -410,7 +410,7 @@ void prot_initialize_replica(Repl_Protocol *rp);
 /* stop protocol session in progress */
 void prot_stop(Repl_Protocol *rp);
 void prot_delete(Repl_Protocol **rpp);
-void prot_free(Repl_Protocol **rpp);
+void prot_free(Repl_Protocol **rpp, int wait_for_done);
 PRBool prot_set_active_protocol (Repl_Protocol *rp, PRBool total);
 void prot_clear_active_protocol (Repl_Protocol *rp);
 Repl_Connection *prot_get_connection(Repl_Protocol *rp);
diff --git a/ldap/servers/plugins/replication/repl5_agmt.c b/ldap/servers/plugins/replication/repl5_agmt.c
index d155004..2571d33 100644
--- a/ldap/servers/plugins/replication/repl5_agmt.c
+++ b/ldap/servers/plugins/replication/repl5_agmt.c
@@ -558,7 +558,7 @@ agmt_start(Repl_Agmt *ra)
     if (ra->protocol != NULL) {
         slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name, "replication already started for agreement \"%s\"\n", agmt_get_long_name(ra));
         PR_Unlock(ra->lock);
-        prot_free(&prot);
+        prot_free(&prot, 0);
         return 0;
     }
 
@@ -606,7 +606,7 @@ windows_agmt_start(Repl_Agmt *ra)
     if (ra->protocol != NULL) {
         slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name, "replication already started for agreement \"%s\"\n", agmt_get_long_name(ra));
         PR_Unlock(ra->lock);
-        prot_free(&prot);
+        prot_free(&prot, 0);
         return 0;
     }
 
@@ -644,8 +644,8 @@ agmt_stop(Repl_Agmt *ra)
 	}
 	PR_Lock(ra->lock);
 	ra->stop_in_progress = PR_FALSE;
-        /* we do not reuse the protocol object so free it */
-        prot_free(&ra->protocol);
+	/* we do not reuse the protocol object so free it */
+	prot_free(&ra->protocol, 1);
 	PR_Unlock(ra->lock);
 	return return_value;
 }
diff --git a/ldap/servers/plugins/replication/repl5_protocol.c b/ldap/servers/plugins/replication/repl5_protocol.c
index 9d8cc16..4a50d0b 100644
--- a/ldap/servers/plugins/replication/repl5_protocol.c
+++ b/ldap/servers/plugins/replication/repl5_protocol.c
@@ -77,6 +77,7 @@ typedef struct repl_protocol
 
 /* States */
 #define STATE_FINISHED 503
+#define STATE_DONE 504
 #define STATE_BAD_STATE_SHOULD_NEVER_HAPPEN 599
 
 /* Forward declarations */
@@ -172,16 +173,39 @@ prot_get_agreement(Repl_Protocol *rp)
 }
 
 
-
-
 void
-prot_free(Repl_Protocol **rpp)
+prot_free(Repl_Protocol **rpp, int wait_for_done)
 {
     Repl_Protocol *rp = NULL;
+    PRIntervalTime interval;
 
     if (rpp == NULL || *rpp == NULL) return;
 
     rp = *rpp;
+    /* 
+     * This function has to wait until prot_thread_main exits if 
+     * prot_start is successfully called and  prot_thread_main is 
+     * running.  Otherwise, we may free Repl_Protocol while it's 
+     * being used.
+     *
+     * This function is supposed to be called when the protocol is 
+     * stopped either after prot_stop is called or when protocol 
+     * hasn't been started.  
+     *
+     * The latter case: prot_free is called with wait_for_done = 0.
+     * The former case: prot_free is called with wait_for_done = 1.
+     * prot_stop had set STATE_FINISHED to next_state and stopped 
+     * the current activity.  But depending upon the threads' 
+     * scheduling, prot_thread_main may not have gotten out of the 
+     * while loop at this moment.  To make sure prot_thread_main 
+     * finished referring Repl_Protocol, we wait for the state set
+     * to STATE_DONE.
+     */
+    interval = PR_MillisecondsToInterval(1000);
+    while (wait_for_done && STATE_DONE != rp->state) 
+    {
+        DS_Sleep(interval);
+    }
 
     PR_Lock(rp->lock);
     if (NULL != rp->prp_incremental)
@@ -220,7 +244,7 @@ prot_delete(Repl_Protocol **rpp)
 	if (NULL != rp)
 	{
 		prot_stop(rp);
-                prot_free(rpp);
+		prot_free(rpp, 1);
 	}
 }
 
@@ -338,6 +362,7 @@ prot_thread_main(void *arg)
 	      }
 	    rp->state = rp->next_state;
 	  }
+	rp->state = STATE_DONE;
 }