Improved "closing" handling. Remove fd from read set when state

changes to closing.  Need to add protection against read set races.

servers/slapd/connection.c

@@ -16,6 +16,18 @@ static Connection *connections = NULL;
 static int conn_index = -1;
 static long conn_nextid = 0;
 
+/* structure state (protected by connections_mutex) */
+#define SLAP_C_UNINITIALIZED	0x0	/* MUST BE ZERO (0) */
+#define SLAP_C_UNUSED			0x1
+#define SLAP_C_USED				0x2
+
+/* connection state (protected by c_mutex ) */
+#define SLAP_C_INVALID			0x0	/* MUST BE ZERO (0) */
+#define SLAP_C_INACTIVE			0x1	/* zero threads */
+#define SLAP_C_ACTIVE			0x2 /* one or more threads */
+#define SLAP_C_BINDING			0x3	/* binding */
+#define SLAP_C_CLOSING			0x4	/* closing */
+
 static Connection* connection_get( int s );
 
 static int connection_input( Connection *c );
@@ -288,6 +300,20 @@ connection_destroy( Connection *c )
    	lber_pvt_sb_destroy( &c->c_sb );
 }
 
+void connection_closing( Connection *c )
+{
+	assert( connections != NULL );
+	assert( c != NULL );
+	assert( c->c_struct_state == SLAP_C_USED );
+	assert( c->c_conn_state != SLAP_C_INVALID );
+
+	if( c->c_conn_state != SLAP_C_CLOSING ) {
+		/* don't listen on this port anymore */
+		slapd_clr_read( c->c_sb.sb_sd, 1 );
+		c->c_conn_state = SLAP_C_CLOSING;
+	}
+}
+
 static void connection_close( Connection *c )
 {
 	assert( connections != NULL );
@@ -483,7 +509,7 @@ connection_operation( void *arg_v )
 	case LDAP_REQ_UNBIND_30:
 #endif
 	case LDAP_REQ_UNBIND:
-		conn->c_conn_state = SLAP_C_CLOSING;
+		connection_closing( conn );
 		break;
 
 	case LDAP_REQ_BIND:
@@ -550,7 +576,7 @@ int connection_read(int s)
 			"connection_read(%d): input error id=%ld, closing.\n",
 			s, c->c_connid, 0 );
 
-		c->c_conn_state = SLAP_C_CLOSING;
+		connection_closing( c );
 		connection_close( c );
 	}
 
@@ -583,7 +609,8 @@ connection_input(
 			"ber_get_next on fd %d failed errno %d (%s)\n",
 			lber_pvt_sb_get_desc(&conn->c_sb), errno,
 			errno > -1 && errno < sys_nerr ?  sys_errlist[errno] : "unknown" );
-		Debug( LDAP_DEBUG_TRACE, "\t*** got %ld of %lu so far\n",
+		Debug( LDAP_DEBUG_TRACE,
+			"\t*** got %ld of %lu so far\n",
 			(long)(conn->c_currentber->ber_rwptr - conn->c_currentber->ber_buf),
 			conn->c_currentber->ber_len, 0 );
 

servers/slapd/daemon.c

@@ -58,9 +58,11 @@ static void slapd_add(int s) {
 	assert( !FD_ISSET( s, &slap_daemon.sd_readers ));
 	assert( !FD_ISSET( s, &slap_daemon.sd_writers ));
 
+#ifndef HAVE_WINSOCK
 	if (s >= slap_daemon.sd_nfds) {
 		slap_daemon.sd_nfds = s + 1;
 	}
+#endif
 
 	FD_SET( s, &slap_daemon.sd_actives );
 	FD_SET( s, &slap_daemon.sd_readers );
@@ -78,7 +80,6 @@ static void slapd_add(int s) {
 void slapd_remove(int s) {
 	ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
 
-	assert( s < slap_daemon.sd_nfds );
 	assert( FD_ISSET( s, &slap_daemon.sd_actives ));
 
 	Debug( LDAP_DEBUG_CONNS, "daemon: removing %d%s%s\n", s,
@@ -95,10 +96,8 @@ void slapd_remove(int s) {
 void slapd_clr_write(int s, int wake) {
 	ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
 
-	assert( s < slap_daemon.sd_nfds );
 	assert( FD_ISSET( s, &slap_daemon.sd_actives) );
-	assert( FD_ISSET( s, &slap_daemon.sd_writers) );
-	FD_SET( s, &slap_daemon.sd_writers );
+	FD_CLR( s, &slap_daemon.sd_writers );
 
 	ldap_pvt_thread_mutex_unlock( &slap_daemon.sd_mutex );
 
@@ -110,6 +109,7 @@ void slapd_clr_write(int s, int wake) {
 void slapd_set_write(int s, int wake) {
 	ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
 
+	assert( FD_ISSET( s, &slap_daemon.sd_actives) );
 	FD_SET( s, &slap_daemon.sd_writers );
 
 	ldap_pvt_thread_mutex_unlock( &slap_daemon.sd_mutex );
@@ -119,6 +119,32 @@ void slapd_set_write(int s, int wake) {
 	}
 }
 
+void slapd_clr_read(int s, int wake) {
+	ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
+
+	assert( FD_ISSET( s, &slap_daemon.sd_actives) );
+	FD_CLR( s, &slap_daemon.sd_readers );
+
+	ldap_pvt_thread_mutex_unlock( &slap_daemon.sd_mutex );
+
+	if( wake ) {
+		ldap_pvt_thread_kill( listener_tid, LDAP_SIGUSR1 );
+	}
+}
+
+void slapd_set_read(int s, int wake) {
+	ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
+
+	assert( FD_ISSET( s, &slap_daemon.sd_actives) );
+	FD_SET( s, &slap_daemon.sd_readers );
+
+	ldap_pvt_thread_mutex_unlock( &slap_daemon.sd_mutex );
+
+	if( wake ) {
+		ldap_pvt_thread_kill( listener_tid, LDAP_SIGUSR1 );
+	}
+}
+
 static void slapd_close(int s) {
 	slapd_remove(s);
 

servers/slapd/proto-slap.h

@@ -115,6 +115,8 @@ long connection_init LDAP_P((
 	int s,
 	const char* name, const char* addr));
 
+void connection_closing LDAP_P(( Connection *c ));
+
 int connection_write LDAP_P((int s));
 int connection_read LDAP_P((int s));
 
@@ -310,6 +312,11 @@ extern int	slap_destroy LDAP_P((void));
 struct sockaddr_in;
 extern int slapd_daemon LDAP_P((struct sockaddr_in *addr));
 
+extern void slapd_set_write LDAP_P((int s, int wake));
+extern void slapd_clr_write LDAP_P((int s, int wake));
+extern void slapd_set_read LDAP_P((int s, int wake));
+extern void slapd_clr_read LDAP_P((int s, int wake));
+
 extern void	slap_set_shutdown LDAP_P((int sig));
 extern void	slap_do_nothing   LDAP_P((int sig));
 

servers/slapd/result.c

@@ -104,7 +104,7 @@ send_ldap_result2(
 		    : "unknown", 0 );
 
 		if ( errno != EWOULDBLOCK && errno != EAGAIN ) {
-			conn->c_conn_state = SLAP_C_CLOSING;
+			connection_closing( conn );
 
 			ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
 			ldap_pvt_thread_mutex_unlock( &conn->c_write_mutex );
@@ -334,7 +334,7 @@ send_search_entry(
 		    : "unknown", 0 );
 
 		if ( errno != EWOULDBLOCK && errno != EAGAIN ) {
-			conn->c_conn_state = SLAP_C_CLOSING;
+			connection_closing( conn );
 
 			ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
 			ldap_pvt_thread_mutex_unlock( &conn->c_write_mutex );

servers/slapd/slap.h

@@ -414,19 +414,6 @@ typedef struct slap_op {
  * represents a connection from an ldap client
  */
 
- 
-/* structure state (protected by connections_mutex) */
-#define SLAP_C_UNINITIALIZED	0x0	/* MUST BE ZERO (0) */
-#define SLAP_C_UNUSED			0x1
-#define SLAP_C_USED				0x2
-
-/* connection state (protected by c_mutex ) */
-#define SLAP_C_INVALID			0x0	/* MUST BE ZERO (0) */
-#define SLAP_C_INACTIVE			0x1	/* zero threads */
-#define SLAP_C_ACTIVE			0x2 /* one or more threads */
-#define SLAP_C_BINDING			0x3	/* binding */
-#define SLAP_C_CLOSING			0x4	/* closing */
-
 typedef struct slap_conn {
 	int			c_struct_state; /* structure management state */
 	int			c_conn_state;	/* connection state */