Przeglądaj źródła

Ticket #48122 nunc-stans FD leak

https://fedorahosted.org/389/ticket/48122
Reviewed by: mreynolds (Thanks!)
Branch: master
Fix Description: Added a lot of debugging when using CONNS level.  Note
that it alters the timing considerably, so it should only be used when
trying to get a general trace of the code, not when performance is an
issue at all.
Instead of scheduling a closure job when the connection is made
readable, wait until the refcnt drops to 1 and the connection is in
the CLOSING state to avoid having dueling closure jobs.
The timeout interval for closure jobs is now the same as the old
slapi_wakeup_timer to avoid tight loops.
Instead of relying on more_data to still be true at the end of
connection_threadmain, check the buffer directly to see if there
is any buffered data, because another thread may have drained the
buffer out from under this thread.   Added a new convenience
function conn_buffered_data_avail_nolock() because the buffer size
is checked in multiple places.
Do not bypass poll in connection_threadmain when using nunc-stans
because it kills performance, spending a lot of time doing the
poll() in connection_read_operation().  Similarly in
ns_handle_new_connection - do not call ns_handle_pr_read_ready, but
instead put the new connection back into nunc-stans and only notify
ns_handle_pr_read_ready when the new connection is ready for read.
In some cases, the new connection is not ready for read right away,
and would end up in the poll() in connection_read_operation().
Platforms tested: Fedora 20, EL7
Flag Day: no
Doc impact: no
Rich Megginson 10 lat temu
rodzic
commit
ba4948f7e6

+ 77 - 10
ldap/servers/slapd/connection.c

@@ -731,7 +731,7 @@ connection_dispatch_operation(Connection *conn, Operation *op, Slapi_PBlock *pb)
 }
 }
 
 
 /* this function should be called under c_mutex */
 /* this function should be called under c_mutex */
-int connection_release_nolock (Connection *conn)
+int connection_release_nolock_ext (Connection *conn, int release_only)
 {
 {
     if (conn->c_refcnt <= 0)
     if (conn->c_refcnt <= 0)
     {
     {
@@ -745,10 +745,21 @@ int connection_release_nolock (Connection *conn)
     {
     {
         conn->c_refcnt--;
         conn->c_refcnt--;
 
 
+        if (!release_only && (conn->c_refcnt == 1) && (conn->c_flags & CONN_FLAG_CLOSING)) {
+        	/* if refcnt == 1 usually means only the active connection list has a ref */
+        	/* refcnt == 0 means conntable just dropped the last ref */
+        	ns_connection_post_io_or_closing(conn);
+        }
+
         return 0;
         return 0;
     }
     }
 }
 }
 
 
+int connection_release_nolock (Connection *conn)
+{
+	return connection_release_nolock_ext(conn, 0);
+}
+
 /* this function should be called under c_mutex */
 /* this function should be called under c_mutex */
 int connection_acquire_nolock_ext (Connection *conn, int allow_when_closing)
 int connection_acquire_nolock_ext (Connection *conn, int allow_when_closing)
 {
 {
@@ -1961,6 +1972,12 @@ connection_read_ldap_data(Connection *conn, PRInt32 *err)
 	return ret;
 	return ret;
 }
 }
 
 
+static size_t
+conn_buffered_data_avail_nolock(Connection *conn)
+{
+	return conn->c_private->c_buffer_bytes - conn->c_private->c_buffer_offset;
+}
+
 /* Upon returning from this function, we have either: 
 /* Upon returning from this function, we have either: 
    1. Read a PDU successfully.
    1. Read a PDU successfully.
    2. Detected some error condition with the connection which requires closing it.
    2. Detected some error condition with the connection which requires closing it.
@@ -1980,6 +1997,7 @@ int connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, i
 	char *buffer = conn->c_private->c_buffer;
 	char *buffer = conn->c_private->c_buffer;
 	PRErrorCode err = 0;
 	PRErrorCode err = 0;
 	PRInt32 syserr = 0;
 	PRInt32 syserr = 0;
+	size_t buffer_data_avail;
 
 
 	PR_Lock(conn->c_mutex);
 	PR_Lock(conn->c_mutex);
 	/*
 	/*
@@ -1995,12 +2013,11 @@ int connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, i
 	
 	
 	*tag = LBER_DEFAULT;
 	*tag = LBER_DEFAULT;
 	/* First check to see if we have buffered data from "before" */
 	/* First check to see if we have buffered data from "before" */
-	if (conn->c_private->c_buffer_bytes - conn->c_private->c_buffer_offset) {
+	if ((buffer_data_avail = conn_buffered_data_avail_nolock(conn))) {
 		/* If so, use that data first */
 		/* If so, use that data first */
 		if ( 0 != get_next_from_buffer( buffer
 		if ( 0 != get_next_from_buffer( buffer
 				+ conn->c_private->c_buffer_offset,
 				+ conn->c_private->c_buffer_offset,
-				conn->c_private->c_buffer_bytes
-				- conn->c_private->c_buffer_offset,
+				buffer_data_avail,
 				&len, tag, op->o_ber, conn )) {
 				&len, tag, op->o_ber, conn )) {
 			ret = CONN_DONE;
 			ret = CONN_DONE;
 			goto done;
 			goto done;
@@ -2011,7 +2028,7 @@ int connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, i
 	while (*tag == LBER_DEFAULT) {
 	while (*tag == LBER_DEFAULT) {
 		int ioblocktimeout_waits = config_get_ioblocktimeout() / CONN_TURBO_TIMEOUT_INTERVAL;
 		int ioblocktimeout_waits = config_get_ioblocktimeout() / CONN_TURBO_TIMEOUT_INTERVAL;
 		/* We should never get here with data remaining in the buffer */
 		/* We should never get here with data remaining in the buffer */
-		PR_ASSERT( !new_operation || 0 == (conn->c_private->c_buffer_bytes - conn->c_private->c_buffer_offset) );
+		PR_ASSERT( !new_operation || 0 == conn_buffered_data_avail_nolock(conn) );
 		/* We make a non-blocking read call */
 		/* We make a non-blocking read call */
 		if (CONNECTION_BUFFER_OFF != conn->c_private->use_buffer) {
 		if (CONNECTION_BUFFER_OFF != conn->c_private->use_buffer) {
 			ret = connection_read_ldap_data(conn,&err);
 			ret = connection_read_ldap_data(conn,&err);
@@ -2088,6 +2105,8 @@ int connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, i
 					ret = CONN_DONE;
 					ret = CONN_DONE;
 					goto done;
 					goto done;
 				}
 				}
+				LDAPDebug( LDAP_DEBUG_CONNS,
+					   "connection %" NSPRIu64 " waited %d times for read to be ready\n", conn->c_connid, waits_done, 0 );
 			} else {
 			} else {
 				/* Some other error, typically meaning bad stuff */
 				/* Some other error, typically meaning bad stuff */
 					syserr = PR_GetOSError();
 					syserr = PR_GetOSError();
@@ -2112,6 +2131,8 @@ int connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, i
 					goto done;
 					goto done;
 				}
 				}
 			}
 			}
+			LDAPDebug( LDAP_DEBUG_CONNS,
+				   "connection %" NSPRIu64 " read %d bytes\n", conn->c_connid, ret, 0 );
 
 
 			new_operation = 0;
 			new_operation = 0;
 			ret = CONN_FOUND_WORK_TO_DO;
 			ret = CONN_FOUND_WORK_TO_DO;
@@ -2119,7 +2140,7 @@ int connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, i
 		}
 		}
 	}
 	}
 	/* If there is remaining buffered data, set the flag to tell the caller */
 	/* If there is remaining buffered data, set the flag to tell the caller */
-	if (conn->c_private->c_buffer_bytes - conn->c_private->c_buffer_offset) {
+	if (conn_buffered_data_avail_nolock(conn)) {
 		*remaining_data = 1;
 		*remaining_data = 1;
 	}
 	}
 
 
@@ -2184,7 +2205,10 @@ void connection_make_readable_nolock(Connection *conn)
 	conn->c_gettingber = 0;
 	conn->c_gettingber = 0;
 	LDAPDebug2Args(LDAP_DEBUG_CONNS, "making readable conn %" NSPRIu64 " fd=%d\n",
 	LDAPDebug2Args(LDAP_DEBUG_CONNS, "making readable conn %" NSPRIu64 " fd=%d\n",
 		       conn->c_connid, conn->c_sd);
 		       conn->c_connid, conn->c_sd);
-	ns_connection_post_io_or_closing(conn);
+	if (!(conn->c_flags & CONN_FLAG_CLOSING)) {
+		/* if the connection is closing, try the close in connection_release_nolock */
+		ns_connection_post_io_or_closing(conn);
+	}
 }
 }
 
 
 /*
 /*
@@ -2333,7 +2357,9 @@ connection_threadmain()
 	int replication_connection = 0; /* If this connection is from a replication supplier, we want to ensure that operation processing is serialized */
 	int replication_connection = 0; /* If this connection is from a replication supplier, we want to ensure that operation processing is serialized */
 	int doshutdown = 0;
 	int doshutdown = 0;
 	int maxthreads = 0;
 	int maxthreads = 0;
+#if !defined(ENABLE_NUNC_STANS) || ENABLE_NUNC_STANS == 0
 	long bypasspollcnt = 0;
 	long bypasspollcnt = 0;
+#endif
 
 
 #if defined( OSF1 ) || defined( hpux )
 #if defined( OSF1 ) || defined( hpux )
 	/* Arrange to ignore SIGPIPE signals. */
 	/* Arrange to ignore SIGPIPE signals. */
@@ -2425,6 +2451,17 @@ connection_threadmain()
 		maxthreads = config_get_maxthreadsperconn();
 		maxthreads = config_get_maxthreadsperconn();
 		more_data = 0;
 		more_data = 0;
 		ret = connection_read_operation(conn,op,&tag,&more_data);
 		ret = connection_read_operation(conn,op,&tag,&more_data);
+		if ((ret == CONN_DONE) || (ret == CONN_TIMEDOUT)) {
+			slapi_log_error(SLAPI_LOG_CONNS, "connection_threadmain",
+					"conn %" NSPRIu64 " read not ready due to %d - thread_turbo_flag %d more_data %d "
+					"ops_initiated %d refcnt %d flags %d\n", conn->c_connid, ret, thread_turbo_flag, more_data,
+					conn->c_opsinitiated, conn->c_refcnt, conn->c_flags);
+		} else if (ret == CONN_FOUND_WORK_TO_DO) {
+			slapi_log_error(SLAPI_LOG_CONNS, "connection_threadmain",
+					"conn %" NSPRIu64 " read operation successfully - thread_turbo_flag %d more_data %d "
+					"ops_initiated %d refcnt %d flags %d\n", conn->c_connid, thread_turbo_flag, more_data,
+					conn->c_opsinitiated, conn->c_refcnt, conn->c_flags);
+		}
 
 
 		curtime = current_time();
 		curtime = current_time();
 #define DB_PERF_TURBO 1		
 #define DB_PERF_TURBO 1		
@@ -2457,11 +2494,21 @@ connection_threadmain()
 			case CONN_TIMEDOUT:
 			case CONN_TIMEDOUT:
 				thread_turbo_flag = 0;
 				thread_turbo_flag = 0;
 				is_timedout = 1;
 				is_timedout = 1;
+				/* In the case of CONN_DONE, more_data could have been set to 1
+				 * in connection_read_operation before an error was encountered.
+				 * In that case, we need to set more_data to 0 - even if there is
+				 * more data available, we're not going to use it anyway.
+				 * In the case of CONN_TIMEDOUT, it is only used in one place, and
+				 * more_data will never be set to 1, so it is safe to set it to 0 here.
+				 * We need more_data to be 0 so the connection will be processed
+				 * correctly at the end of this function.
+				 */
+				more_data = 0;
 				/* note: 
 				/* note: 
 				 * should call connection_make_readable after the op is removed
 				 * should call connection_make_readable after the op is removed
 				 * connection_make_readable(conn);
 				 * connection_make_readable(conn);
 				 */
 				 */
-				LDAPDebug(LDAP_DEBUG_CONNS,"conn %" NSPRIu64 " leaving turbo mode due to %d\n",conn->c_connid,ret,0); 
+				LDAPDebug(LDAP_DEBUG_CONNS,"conn %" NSPRIu64 " leaving turbo mode due to %d\n",conn->c_connid,ret,0);
 				goto done;
 				goto done;
 			case CONN_SHUTDOWN:
 			case CONN_SHUTDOWN:
 				LDAPDebug( LDAP_DEBUG_TRACE, 
 				LDAPDebug( LDAP_DEBUG_TRACE, 
@@ -2499,6 +2546,13 @@ connection_threadmain()
 				/* once the connection is readable, another thread may access conn,
 				/* once the connection is readable, another thread may access conn,
 				 * so need locking from here on */
 				 * so need locking from here on */
 				signal_listner();
 				signal_listner();
+/* with nunc-stans, I see an enormous amount of time spent in the poll() in
+ * connection_read_operation() when the below code is enabled - not sure why
+ * nunc-stans makes such a huge difference - for now, just disable this code
+ * when using nunc-stans - it is supposed to be an optimization but turns out
+ * to not be the opposite with nunc-stans
+ */
+#if !defined(ENABLE_NUNC_STANS) || ENABLE_NUNC_STANS == 0
 			} else { /* more data in conn - just put back on work_q - bypass poll */
 			} else { /* more data in conn - just put back on work_q - bypass poll */
 				bypasspollcnt++;
 				bypasspollcnt++;
 				PR_Lock(conn->c_mutex);
 				PR_Lock(conn->c_mutex);
@@ -2511,11 +2565,13 @@ connection_threadmain()
 					 */
 					 */
 					conn->c_idlesince = curtime;
 					conn->c_idlesince = curtime;
 					connection_activity(conn, maxthreads);
 					connection_activity(conn, maxthreads);
+					LDAPDebug(LDAP_DEBUG_CONNS,"conn %" NSPRIu64 " queued because more_data\n",conn->c_connid,0,0);
 				} else {
 				} else {
 					/* keep count of how many times maxthreads has blocked an operation */
 					/* keep count of how many times maxthreads has blocked an operation */
 					conn->c_maxthreadsblocked++;
 					conn->c_maxthreadsblocked++;
 				}
 				}
 				PR_Unlock(conn->c_mutex);
 				PR_Unlock(conn->c_mutex);
+#endif
 			}
 			}
 		}
 		}
 
 
@@ -2589,6 +2645,9 @@ done:
 			connection_remove_operation_ext( pb, conn, op );
 			connection_remove_operation_ext( pb, conn, op );
 
 
 			/* If we're in turbo mode, we keep our reference to the connection alive */
 			/* If we're in turbo mode, we keep our reference to the connection alive */
+			/* can't use the more_data var because connection could have changed in another thread */
+			more_data = conn_buffered_data_avail_nolock(conn) ? 1 : 0;
+			LDAPDebug(LDAP_DEBUG_CONNS,"conn %" NSPRIu64 " check more_data %d thread_turbo_flag %d\n",conn->c_connid,more_data,thread_turbo_flag);
 			if (!more_data) {
 			if (!more_data) {
 				if (!thread_turbo_flag) {
 				if (!thread_turbo_flag) {
 					/*
 					/*
@@ -3011,7 +3070,7 @@ static ps_wakeup_all_fn_ptr ps_wakeup_all_fn = NULL;
  */
  */
 
 
 void
 void
-disconnect_server_nomutex( Connection *conn, PRUint64 opconnid, int opid, PRErrorCode reason, PRInt32 error )
+disconnect_server_nomutex_ext( Connection *conn, PRUint64 opconnid, int opid, PRErrorCode reason, PRInt32 error, int schedule_closure_job )
 {
 {
     if ( ( conn->c_sd != SLAPD_INVALID_SOCKET &&
     if ( ( conn->c_sd != SLAPD_INVALID_SOCKET &&
 	    conn->c_connid == opconnid ) && !(conn->c_flags & CONN_FLAG_CLOSING) )
 	    conn->c_connid == opconnid ) && !(conn->c_flags & CONN_FLAG_CLOSING) )
@@ -3088,7 +3147,9 @@ disconnect_server_nomutex( Connection *conn, PRUint64 opconnid, int opid, PRErro
 				}
 				}
 			}
 			}
 		}
 		}
-		ns_connection_post_io_or_closing(conn); /* make sure event loop wakes up and closes this conn */
+		if (schedule_closure_job) {
+			ns_connection_post_io_or_closing(conn); /* make sure event loop wakes up and closes this conn */
+		}
 
 
     } else {
     } else {
 	    LDAPDebug2Args(LDAP_DEBUG_CONNS, "not setting conn %d to be disconnected: %s\n",
 	    LDAPDebug2Args(LDAP_DEBUG_CONNS, "not setting conn %d to be disconnected: %s\n",
@@ -3099,6 +3160,12 @@ disconnect_server_nomutex( Connection *conn, PRUint64 opconnid, int opid, PRErro
     }
     }
 }
 }
 
 
+void
+disconnect_server_nomutex( Connection *conn, PRUint64 opconnid, int opid, PRErrorCode reason, PRInt32 error )
+{
+	disconnect_server_nomutex_ext(conn, opconnid, opid, reason, error, 1);
+}
+
 void
 void
 connection_abandon_operations( Connection *c )
 connection_abandon_operations( Connection *c )
 {
 {

+ 1 - 1
ldap/servers/slapd/conntable.c

@@ -354,7 +354,7 @@ connection_table_move_connection_on_to_active_list(Connection_Table *ct,Connecti
 
 
 	PR_Lock(ct->table_mutex);
 	PR_Lock(ct->table_mutex);
 
 
-	connection_acquire_nolock (c);
+	PR_ASSERT(0 == connection_acquire_nolock (c));
 
 
 #ifdef FOR_DEBUGGING
 #ifdef FOR_DEBUGGING
     slapi_log_error(SLAPI_LOG_FATAL, "connection", "Moving connection into active list\n");
     slapi_log_error(SLAPI_LOG_FATAL, "connection", "Moving connection into active list\n");

+ 56 - 36
ldap/servers/slapd/daemon.c

@@ -2402,6 +2402,25 @@ handle_pr_read_ready(Connection_Table *ct, PRIntn num_poll)
 }
 }
 
 
 #ifdef ENABLE_NUNC_STANS
 #ifdef ENABLE_NUNC_STANS
+#define CONN_NEEDS_CLOSING(c) (c->c_flags & CONN_FLAG_CLOSING) || (c->c_sd == SLAPD_INVALID_SOCKET)
+/* Used internally by ns_handle_closure and ns_handle_pr_read_ready.
+ * Returns 0 if the connection was successfully closed, or 1 otherwise.
+ * Must be called with the c->c_mutex locked.
+ */
+static int
+ns_handle_closure_nomutex(Connection *c)
+{
+	int rc = 0;
+	PR_ASSERT(c->c_refcnt > 0); /* one for the conn active list, plus possible other threads */
+	PR_ASSERT(CONN_NEEDS_CLOSING(c));
+	if (connection_table_move_connection_out_of_active_list(c->c_ct, c)) {
+		/* not closed - another thread still has a ref */
+		rc = 1;
+		/* reschedule closure job */
+		ns_connection_post_io_or_closing(c);
+	}
+	return rc;
+}
 /* This function is called when the connection has been marked
 /* This function is called when the connection has been marked
  * as closing and needs to be cleaned up.  It will keep trying
  * as closing and needs to be cleaned up.  It will keep trying
  * and re-arming itself until there are no references.
  * and re-arming itself until there are no references.
@@ -2415,22 +2434,20 @@ ns_handle_closure(struct ns_job_t *job)
 	/* this function must be called from the event loop thread */
 	/* this function must be called from the event loop thread */
 	PR_ASSERT(0 == NS_JOB_IS_THREAD(ns_job_get_type(job)));
 	PR_ASSERT(0 == NS_JOB_IS_THREAD(ns_job_get_type(job)));
 	PR_Lock(c->c_mutex);
 	PR_Lock(c->c_mutex);
-	c->c_ns_close_jobs--;
-	connection_release_nolock(c); /* release ref acquired when job was added */
-	if (connection_table_move_connection_out_of_active_list(c->c_ct, c)) {
-		do_yield = 1;
-		ns_connection_post_io_or_closing(c);
-	}
+	connection_release_nolock_ext(c, 1); /* release ref acquired for event framework */
+	PR_ASSERT(c->c_ns_close_jobs == 1); /* should be exactly 1 active close job - this one */
+	c->c_ns_close_jobs--; /* this job is processing closure */
+	do_yield = ns_handle_closure_nomutex(c);
 	PR_Unlock(c->c_mutex);
 	PR_Unlock(c->c_mutex);
 	ns_job_done(job);
 	ns_job_done(job);
 	if (do_yield) {
 	if (do_yield) {
+		/* closure not done - another reference still outstanding */
 		/* yield thread after unlocking conn mutex */
 		/* yield thread after unlocking conn mutex */
 		PR_Sleep(PR_INTERVAL_NO_WAIT); /* yield to allow other thread to release conn */
 		PR_Sleep(PR_INTERVAL_NO_WAIT); /* yield to allow other thread to release conn */
 	}
 	}
 	return;
 	return;
 }
 }
 #endif
 #endif
-#define CONN_NEEDS_CLOSING(c) (c->c_flags & CONN_FLAG_CLOSING) || (c->c_sd == SLAPD_INVALID_SOCKET)
 
 
 /**
 /**
  * Schedule more I/O for this connection, or make sure that it
  * Schedule more I/O for this connection, or make sure that it
@@ -2441,19 +2458,22 @@ ns_connection_post_io_or_closing(Connection *conn)
 {
 {
 #ifdef ENABLE_NUNC_STANS
 #ifdef ENABLE_NUNC_STANS
 	struct timeval tv;
 	struct timeval tv;
-	ns_job_func_t job_func;
 
 
 	if (CONN_NEEDS_CLOSING(conn)) {
 	if (CONN_NEEDS_CLOSING(conn)) {
+		/* there should only ever be 0 or 1 active closure jobs */
+		PR_ASSERT((conn->c_ns_close_jobs == 0) || (conn->c_ns_close_jobs == 1));
 		if (conn->c_ns_close_jobs) {
 		if (conn->c_ns_close_jobs) {
 			LDAPDebug2Args(LDAP_DEBUG_CONNS, "already a close job in progress on conn %" NSPRIu64 " for fd=%d\n",
 			LDAPDebug2Args(LDAP_DEBUG_CONNS, "already a close job in progress on conn %" NSPRIu64 " for fd=%d\n",
 				       conn->c_connid, conn->c_sd);
 				       conn->c_connid, conn->c_sd);
 			return;
 			return;
 		} else {
 		} else {
-			/* just make sure the event is processed at the next possible event loop run */
+			/* just make sure we schedule the event to be closed in a timely manner */
 			tv.tv_sec = 0;
 			tv.tv_sec = 0;
-			tv.tv_usec = 1000;
-			job_func = ns_handle_closure;
-			conn->c_ns_close_jobs++;
+			tv.tv_usec = slapd_wakeup_timer * 1000;
+			conn->c_ns_close_jobs++; /* now 1 active closure job */
+			connection_acquire_nolock_ext(conn, 1 /* allow acquire even when closing */); /* event framework now has a reference */
+			ns_add_timeout_job(conn->c_tp, &tv, NS_JOB_TIMER,
+					   ns_handle_closure, conn, NULL);
 			LDAPDebug2Args(LDAP_DEBUG_CONNS, "post closure job for conn %" NSPRIu64 " for fd=%d\n",
 			LDAPDebug2Args(LDAP_DEBUG_CONNS, "post closure job for conn %" NSPRIu64 " for fd=%d\n",
 				       conn->c_connid, conn->c_sd);
 				       conn->c_connid, conn->c_sd);
 		}
 		}
@@ -2461,14 +2481,13 @@ ns_connection_post_io_or_closing(Connection *conn)
 		/* process event normally - wait for I/O until idletimeout */
 		/* process event normally - wait for I/O until idletimeout */
 		tv.tv_sec = conn->c_idletimeout;
 		tv.tv_sec = conn->c_idletimeout;
 		tv.tv_usec = 0;
 		tv.tv_usec = 0;
-		job_func = ns_handle_pr_read_ready;
+		PR_ASSERT(0 == connection_acquire_nolock(conn)); /* event framework now has a reference */
+		ns_add_io_timeout_job(conn->c_tp, conn->c_prfd, &tv,
+				      NS_JOB_READ|NS_JOB_PRESERVE_FD,
+				      ns_handle_pr_read_ready, conn, NULL);
 		LDAPDebug2Args(LDAP_DEBUG_CONNS, "post I/O job for conn %" NSPRIu64 " for fd=%d\n",
 		LDAPDebug2Args(LDAP_DEBUG_CONNS, "post I/O job for conn %" NSPRIu64 " for fd=%d\n",
 			       conn->c_connid, conn->c_sd);
 			       conn->c_connid, conn->c_sd);
 	}
 	}
-	connection_acquire_nolock_ext(conn, 1 /* allow acquire even when closing */); /* event framework will have reference */
-	ns_add_io_timeout_job(conn->c_tp, conn->c_prfd, &tv,
-			      NS_JOB_READ|NS_JOB_PRESERVE_FD,
-			      job_func, conn, NULL);
 #endif
 #endif
 }
 }
 
 
@@ -2480,7 +2499,6 @@ ns_connection_post_io_or_closing(Connection *conn)
 void
 void
 ns_handle_pr_read_ready(struct ns_job_t *job)
 ns_handle_pr_read_ready(struct ns_job_t *job)
 {
 {
-	int need_closure = 0;
 	int maxthreads = config_get_maxthreadsperconn();
 	int maxthreads = config_get_maxthreadsperconn();
 	Connection *c = (Connection *)ns_job_get_data(job);
 	Connection *c = (Connection *)ns_job_get_data(job);
 
 
@@ -2492,14 +2510,14 @@ ns_handle_pr_read_ready(struct ns_job_t *job)
 		       c->c_connid, c->c_sd);
 		       c->c_connid, c->c_sd);
 	/* if we were called due to some i/o event, see what the state of the socket is */
 	/* if we were called due to some i/o event, see what the state of the socket is */
 	if (slapi_is_loglevel_set(SLAPI_LOG_CONNS) && !NS_JOB_IS_TIMER(ns_job_get_output_type(job)) && c && c->c_sd) {
 	if (slapi_is_loglevel_set(SLAPI_LOG_CONNS) && !NS_JOB_IS_TIMER(ns_job_get_output_type(job)) && c && c->c_sd) {
-		/* see if socket is closed */
+		/* check socket state */
 		char buf[1];
 		char buf[1];
 		ssize_t rc = recv(c->c_sd, buf, sizeof(buf), MSG_PEEK);
 		ssize_t rc = recv(c->c_sd, buf, sizeof(buf), MSG_PEEK);
 		if (!rc) {
 		if (!rc) {
 			LDAPDebug2Args(LDAP_DEBUG_CONNS, "socket is closed conn %" NSPRIu64 " for fd=%d\n",
 			LDAPDebug2Args(LDAP_DEBUG_CONNS, "socket is closed conn %" NSPRIu64 " for fd=%d\n",
 				       c->c_connid, c->c_sd);
 				       c->c_connid, c->c_sd);
 		} else if (rc > 0) {
 		} else if (rc > 0) {
-			LDAPDebug2Args(LDAP_DEBUG_CONNS, "socket has data available for conn %" NSPRIu64 " for fd=%d\n",
+			LDAPDebug2Args(LDAP_DEBUG_CONNS, "socket read data available for conn %" NSPRIu64 " for fd=%d\n",
 				       c->c_connid, c->c_sd);
 				       c->c_connid, c->c_sd);
 		} else if ((errno == EAGAIN) || (errno == EWOULDBLOCK)) {
 		} else if ((errno == EAGAIN) || (errno == EWOULDBLOCK)) {
 			LDAPDebug2Args(LDAP_DEBUG_CONNS, "socket has no data available conn %" NSPRIu64 " for fd=%d\n",
 			LDAPDebug2Args(LDAP_DEBUG_CONNS, "socket has no data available conn %" NSPRIu64 " for fd=%d\n",
@@ -2509,14 +2527,15 @@ ns_handle_pr_read_ready(struct ns_job_t *job)
 				  errno, c->c_connid, c->c_sd);
 				  errno, c->c_connid, c->c_sd);
 		}
 		}
 	}
 	}
-	connection_release_nolock(c); /* release ref acquired when job was added */
+	connection_release_nolock_ext(c, 1); /* release ref acquired when job was added */
 	if (CONN_NEEDS_CLOSING(c)) {
 	if (CONN_NEEDS_CLOSING(c)) {
-		need_closure = 1;
+		ns_handle_closure_nomutex(c);
 	} else if (NS_JOB_IS_TIMER(ns_job_get_output_type(job))) {
 	} else if (NS_JOB_IS_TIMER(ns_job_get_output_type(job))) {
 		/* idle timeout */
 		/* idle timeout */
-		disconnect_server_nomutex(c, c->c_connid, -1,
-				          SLAPD_DISCONNECT_IDLE_TIMEOUT, EAGAIN);
-		need_closure = 1;
+		disconnect_server_nomutex_ext(c, c->c_connid, -1,
+					      SLAPD_DISCONNECT_IDLE_TIMEOUT, EAGAIN,
+				              0 /* do not schedule closure, do it next */);
+		ns_handle_closure_nomutex(c);
 	} else if ((connection_activity(c, maxthreads)) == -1) {
 	} else if ((connection_activity(c, maxthreads)) == -1) {
 		/* This might happen as a result of
 		/* This might happen as a result of
 		 * trying to acquire a closing connection
 		 * trying to acquire a closing connection
@@ -2525,16 +2544,14 @@ ns_handle_pr_read_ready(struct ns_job_t *job)
 			       " as fd=%d is already closing\n", c->c_connid, c->c_sd);
 			       " as fd=%d is already closing\n", c->c_connid, c->c_sd);
 		/* The call disconnect_server should do nothing,
 		/* The call disconnect_server should do nothing,
 		 * as the connection c should be already set to CLOSING */
 		 * as the connection c should be already set to CLOSING */
-		disconnect_server_nomutex(c, c->c_connid, -1,
-				          SLAPD_DISCONNECT_POLL, EPIPE);
-		need_closure = 1;
+		disconnect_server_nomutex_ext(c, c->c_connid, -1,
+				              SLAPD_DISCONNECT_POLL, EPIPE,
+				              0 /* do not schedule closure, do it next */);
+		ns_handle_closure_nomutex(c);
 	} else {
 	} else {
 		LDAPDebug2Args(LDAP_DEBUG_CONNS, "queued conn %" NSPRIu64 " for fd=%d\n",
 		LDAPDebug2Args(LDAP_DEBUG_CONNS, "queued conn %" NSPRIu64 " for fd=%d\n",
 			       c->c_connid, c->c_sd);
 			       c->c_connid, c->c_sd);
 	}
 	}
-	if (need_closure) {
-		ns_connection_post_io_or_closing(c);
-	}
 	PR_Unlock(c->c_mutex);
 	PR_Unlock(c->c_mutex);
 	ns_job_done(job);
 	ns_job_done(job);
 	return;
 	return;
@@ -3273,12 +3290,15 @@ ns_handle_new_connection(struct ns_job_t *job)
 		}
 		}
 		return;
 		return;
 	}
 	}
-	/* now, set up the conn for reading - no thread - ns_handle_pr_read_ready
-	 * must be run in the event loop thread
-	 */
 	c->c_tp = ns_job_get_tp(job);
 	c->c_tp = ns_job_get_tp(job);
-	connection_acquire_nolock(c); /* event framework now has ref */
-	ns_add_job(ns_job_get_tp(job), NS_JOB_NONE, ns_handle_pr_read_ready, c, NULL);
+	/* This originally just called ns_handle_pr_read_ready directly - however, there
+	 * are certain cases where accept() will return a file descriptor that is not
+	 * immediately available for reading - this would cause the poll() in
+	 * connection_read_operation() to be hit - it seemed to perform better when
+	 * that poll() was avoided, even at the expense of putting this new fd back
+	 * in nunc-stans to poll for read ready.
+	 */
+	ns_connection_post_io_or_closing(c);
 	return;
 	return;
 }
 }
 #endif
 #endif

+ 2 - 0
ldap/servers/slapd/proto-slap.h

@@ -1003,6 +1003,7 @@ int send_ldap_referral( Slapi_PBlock *pb, Slapi_Entry *e, struct berval **refs,
 int send_ldapv3_referral( Slapi_PBlock *pb, struct berval **urls );
 int send_ldapv3_referral( Slapi_PBlock *pb, struct berval **urls );
 int set_db_default_result_handlers(Slapi_PBlock *pb);
 int set_db_default_result_handlers(Slapi_PBlock *pb);
 void disconnect_server_nomutex( Connection *conn, PRUint64 opconnid, int opid, PRErrorCode reason, PRInt32 error );
 void disconnect_server_nomutex( Connection *conn, PRUint64 opconnid, int opid, PRErrorCode reason, PRInt32 error );
+void disconnect_server_nomutex_ext( Connection *conn, PRUint64 opconnid, int opid, PRErrorCode reason, PRInt32 error, int schedule_closure_job );
 long g_get_current_conn_count();
 long g_get_current_conn_count();
 void g_increment_current_conn_count();
 void g_increment_current_conn_count();
 void g_decrement_current_conn_count();
 void g_decrement_current_conn_count();
@@ -1423,6 +1424,7 @@ int mapping_tree_get_extension_type ();
 int connection_acquire_nolock (Connection *conn);
 int connection_acquire_nolock (Connection *conn);
 int connection_acquire_nolock_ext (Connection *conn, int allow_when_closing);
 int connection_acquire_nolock_ext (Connection *conn, int allow_when_closing);
 int connection_release_nolock (Connection *conn);
 int connection_release_nolock (Connection *conn);
+int connection_release_nolock_ext (Connection *conn, int release_only);
 int connection_is_free (Connection *conn);
 int connection_is_free (Connection *conn);
 int connection_is_active_nolock (Connection *conn);
 int connection_is_active_nolock (Connection *conn);
 #if defined(USE_OPENLDAP)
 #if defined(USE_OPENLDAP)