From: Hallvard Furuseth <hallvard@openldap.org>
Date: Thu, 31 May 2007 12:47:24 +0000 (+0000)
Subject: In pool_submit():
X-Git-Tag: OPENLDAP_REL_ENG_2_4_MP~439
X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=1b15fa9c6e2b8b553dec255584067ff86030fb52;p=openldap

In pool_submit():
- Move problematic mutex_unlock()-mutex_lock() into the unused #ifdef
  LDAP_PVT_THREAD_POOL_SEM_LOAD_CONTROL and add a FIXME comment inside.
- Delay ltp_starting--; until pool_wrapper().  Nonzero value could otherwise
  no longer be exposed when !defined LDAP_PVT_THREAD_POOL_SEM_LOAD_CONTROL.
---

diff --git a/libraries/libldap_r/tpool.c b/libraries/libldap_r/tpool.c
index ac4f791432..c3e5686ca4 100644
--- a/libraries/libldap_r/tpool.c
+++ b/libraries/libldap_r/tpool.c
@@ -393,24 +393,27 @@ ldap_pvt_thread_pool_submit (
 		pool->ltp_starting++;
 		need_thread = 1;
 	}
-	ldap_pvt_thread_mutex_unlock(&pool->ltp_mutex);
 
 #ifdef LDAP_PVT_THREAD_POOL_SEM_LOAD_CONTROL
+	ldap_pvt_thread_mutex_unlock(&pool->ltp_mutex);
 	ldap_lazy_sem_op_submit( thread_pool_sem );
+	/* FIXME:  Another thread can now handle and release ctx, after
+	 * which a newly submitted op can reuse ctx.  Then it is wrong for
+	 * the "if (pool->ltp_open_count == 0)" code below to release ctx.
+	 */
 #endif
 
 	if (need_thread) {
-		int rc;
-
-		ldap_pvt_thread_mutex_lock(&pool->ltp_mutex);
-
-		rc = ldap_pvt_thread_create( &thr, 1,
-			ldap_int_thread_pool_wrapper, pool );
-		pool->ltp_starting--;
-		if (rc != 0) {
+		if (0 != ldap_pvt_thread_create(
+			&thr, 1, ldap_int_thread_pool_wrapper, pool))
+		{
 			/* couldn't create thread.  back out of
 			 * ltp_open_count and check for even worse things.
 			 */
+#ifdef LDAP_PVT_THREAD_POOL_SEM_LOAD_CONTROL
+			ldap_pvt_thread_mutex_lock(&pool->ltp_mutex);
+#endif
+			pool->ltp_starting--;
 			pool->ltp_open_count--;
 			if (pool->ltp_open_count == 0) {
 				/* no open threads at all?!?
@@ -435,15 +438,21 @@ ldap_pvt_thread_pool_submit (
 					return(-1);
 				}
 			}
+#ifdef LDAP_PVT_THREAD_POOL_SEM_LOAD_CONTROL
+			ldap_pvt_thread_mutex_unlock(&pool->ltp_mutex);
+#endif
 			/* there is another open thread, so this
 			 * context will be handled eventually.
 			 * continue on, we have signalled that
 			 * the context is waiting.
 			 */
 		}
-		ldap_pvt_thread_mutex_unlock(&pool->ltp_mutex);
 	}
 
+#ifndef LDAP_PVT_THREAD_POOL_SEM_LOAD_CONTROL
+	ldap_pvt_thread_mutex_unlock(&pool->ltp_mutex);
+#endif
+
 	return(0);
 }
 
@@ -675,6 +684,8 @@ ldap_int_thread_pool_wrapper (
 	thread_keys[keyslot].ctx = &uctx;
 	ldap_pvt_thread_mutex_unlock(&ldap_pvt_thread_pool_mutex);
 
+	pool->ltp_starting--;
+
 	for (;;) {
 		while (pool->ltp_pause)
 			ldap_pvt_thread_cond_wait(&pool->ltp_cond, &pool->ltp_mutex);