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(a)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;
}