ITS#9498 More connection streamlining

Since c_mutex is now always inited at startup time, we no longer
need connections_mutex to protect from uninit'd structures

servers/slapd/connection.c

@@ -47,8 +47,6 @@
 #include "slapi/slapi.h"
 #endif
 
-/* protected by connections_mutex */
-static ldap_pvt_thread_mutex_t connections_mutex;
 static Connection *connections = NULL;
 
 static ldap_pvt_thread_mutex_t conn_nextid_mutex;
@@ -107,7 +105,6 @@ int connections_init(void)
 	}
 
 	/* should check return of every call */
-	ldap_pvt_thread_mutex_init( &connections_mutex );
 	ldap_pvt_thread_mutex_init( &conn_nextid_mutex );
 
 	connections = (Connection *) ch_calloc( dtblsize, sizeof(Connection) );
@@ -119,9 +116,6 @@ int connections_init(void)
 		return -1;
 	}
 
-	assert( connections[0].c_struct_state == SLAP_C_UNINITIALIZED );
-	assert( connections[dtblsize-1].c_struct_state == SLAP_C_UNINITIALIZED );
-
 	for (i=0; i<dtblsize; i++) {
 		connections[i].c_conn_idx = i;
 		ldap_pvt_thread_mutex_init( &connections[i].c_mutex );
@@ -154,11 +148,11 @@ int connections_destroy(void)
 	}
 
 	for ( i = 0; i < dtblsize; i++ ) {
-		if( connections[i].c_struct_state != SLAP_C_UNINITIALIZED ) {
+		ldap_pvt_thread_mutex_destroy( &connections[i].c_mutex );
+		ldap_pvt_thread_mutex_destroy( &connections[i].c_write1_mutex );
+		ldap_pvt_thread_cond_destroy( &connections[i].c_write1_cv );
+		if( connections[i].c_sb ) {
 			ber_sockbuf_free( connections[i].c_sb );
-			ldap_pvt_thread_mutex_destroy( &connections[i].c_mutex );
-			ldap_pvt_thread_mutex_destroy( &connections[i].c_write1_mutex );
-			ldap_pvt_thread_cond_destroy( &connections[i].c_write1_cv );
 #ifdef LDAP_SLAPI
 			if ( slapi_plugins_used ) {
 				slapi_int_free_object_extensions( SLAPI_X_EXT_CONNECTION,
@@ -171,7 +165,6 @@ int connections_destroy(void)
 	free( connections );
 	connections = NULL;
 
-	ldap_pvt_thread_mutex_destroy( &connections_mutex );
 	ldap_pvt_thread_mutex_destroy( &conn_nextid_mutex );
 	return 0;
 }
@@ -184,22 +177,20 @@ int connections_shutdown(void)
 	ber_socket_t i;
 
 	for ( i = 0; i < dtblsize; i++ ) {
-		if( connections[i].c_struct_state != SLAP_C_UNINITIALIZED ) {
-			ldap_pvt_thread_mutex_lock( &connections[i].c_mutex );
-			if( connections[i].c_struct_state == SLAP_C_USED ) {
-
-				/* give persistent clients a chance to cleanup */
-				if( connections[i].c_conn_state == SLAP_C_CLIENT ) {
-					ldap_pvt_thread_pool_submit( &connection_pool,
-					connections[i].c_clientfunc, connections[i].c_clientarg );
-				} else {
-					/* c_mutex is locked */
-					connection_closing( &connections[i], "slapd shutdown" );
-					connection_close( &connections[i] );
-				}
+		ldap_pvt_thread_mutex_lock( &connections[i].c_mutex );
+		if( connections[i].c_conn_state > SLAP_C_INVALID ) {
+
+			/* give persistent clients a chance to cleanup */
+			if( connections[i].c_conn_state == SLAP_C_CLIENT ) {
+				ldap_pvt_thread_pool_submit( &connection_pool,
+				connections[i].c_clientfunc, connections[i].c_clientarg );
+			} else {
+				/* c_mutex is locked */
+				connection_closing( &connections[i], "slapd shutdown" );
+				connection_close( &connections[i] );
 			}
-			ldap_pvt_thread_mutex_unlock( &connections[i].c_mutex );
 		}
+		ldap_pvt_thread_mutex_unlock( &connections[i].c_mutex );
 	}
 
 	return 0;
@@ -281,15 +272,12 @@ static Connection* connection_get( ber_socket_t s )
 	if( c != NULL ) {
 		ldap_pvt_thread_mutex_lock( &c->c_mutex );
 
-		assert( c->c_struct_state != SLAP_C_UNINITIALIZED );
-
-		if( c->c_struct_state != SLAP_C_USED ) {
+		if( c->c_conn_state == SLAP_C_INVALID ) {
 			/* connection must have been closed due to resched */
 
 			Debug( LDAP_DEBUG_CONNS,
 				"connection_get(%d): connection not used\n",
 				s );
-			assert( c->c_conn_state == SLAP_C_INVALID );
 			assert( c->c_sd == AC_SOCKET_INVALID );
 
 			ldap_pvt_thread_mutex_unlock( &c->c_mutex );
@@ -302,7 +290,6 @@ static Connection* connection_get( ber_socket_t s )
 
 		c->c_n_get++;
 
-		assert( c->c_struct_state == SLAP_C_USED );
 		assert( c->c_conn_state != SLAP_C_INVALID );
 		assert( c->c_sd != AC_SOCKET_INVALID );
 
@@ -329,7 +316,6 @@ Connection * connection_init(
 {
 	unsigned long id;
 	Connection *c;
-	int doinit = 0;
 	ber_socket_t sfd = SLAP_FD2SOCK(s);
 
 	assert( connections != NULL );
@@ -353,13 +339,8 @@ Connection * connection_init(
 	c = &connections[s];
 
 	ldap_pvt_thread_mutex_lock( &c->c_mutex );
-	if( c->c_struct_state == SLAP_C_UNINITIALIZED ) {
-		doinit = 1;
-	} else {
-		assert( c->c_struct_state == SLAP_C_UNUSED );
-	}
 
-	if( doinit ) {
+	if( !c->c_sb ) {
 		c->c_send_ldap_result = slap_send_ldap_result;
 		c->c_send_search_entry = slap_send_search_entry;
 		c->c_send_search_reference = slap_send_search_reference;
@@ -432,10 +413,7 @@ Connection * connection_init(
 
 	if ( flags & CONN_IS_CLIENT ) {
 		c->c_connid = 0;
-		ldap_pvt_thread_mutex_lock( &connections_mutex );
 		c->c_conn_state = SLAP_C_CLIENT;
-		c->c_struct_state = SLAP_C_USED;
-		ldap_pvt_thread_mutex_unlock( &connections_mutex );
 		c->c_close_reason = "?";			/* should never be needed */
 		ber_sockbuf_ctrl( c->c_sb, LBER_SB_OPT_SET_FD, &sfd );
 		ldap_pvt_thread_mutex_unlock( &c->c_mutex );
@@ -536,10 +514,7 @@ Connection * connection_init(
 	id = c->c_connid = conn_nextid++;
 	ldap_pvt_thread_mutex_unlock( &conn_nextid_mutex );
 
-	ldap_pvt_thread_mutex_lock( &connections_mutex );
 	c->c_conn_state = SLAP_C_INACTIVE;
-	c->c_struct_state = SLAP_C_USED;
-	ldap_pvt_thread_mutex_unlock( &connections_mutex );
 	c->c_close_reason = "?";			/* should never be needed */
 
 	c->c_ssf = c->c_transport_ssf = ssf;
@@ -614,7 +589,6 @@ connection_destroy( Connection *c )
 
 	assert( connections != NULL );
 	assert( c != NULL );
-	assert( c->c_struct_state != SLAP_C_UNUSED );
 	assert( c->c_conn_state != SLAP_C_INVALID );
 	assert( LDAP_STAILQ_EMPTY(&c->c_ops) );
 	assert( LDAP_STAILQ_EMPTY(&c->c_pending_ops) );
@@ -628,10 +602,6 @@ connection_destroy( Connection *c )
 	connid = c->c_connid;
 	close_reason = c->c_close_reason;
 
-	ldap_pvt_thread_mutex_lock( &connections_mutex );
-	c->c_struct_state = SLAP_C_PENDING;
-	ldap_pvt_thread_mutex_unlock( &connections_mutex );
-
 	backend_connection_destroy(c);
 
 	c->c_protocol = 0;
@@ -683,7 +653,6 @@ connection_destroy( Connection *c )
 		ber_sockbuf_ctrl( c->c_sb, LBER_SB_OPT_SET_MAX_INCOMING, &max );
 	}
 	c->c_conn_state = SLAP_C_INVALID;
-	c->c_struct_state = SLAP_C_UNUSED;
 
 	/* c must be fully reset by this point; when we call slapd_remove
 	 * it may get immediately reused by a new connection.
@@ -718,8 +687,7 @@ int connection_valid( Connection *c )
 
 	assert( c != NULL );
 
-	return c->c_struct_state == SLAP_C_USED &&
-		c->c_conn_state >= SLAP_C_ACTIVE &&
+	return c->c_conn_state >= SLAP_C_ACTIVE &&
 		c->c_conn_state <= SLAP_C_CLIENT;
 }
 
@@ -795,8 +763,6 @@ void connection_closing( Connection *c, const char *why )
 	assert( connections != NULL );
 	assert( c != NULL );
 
-	if ( c->c_struct_state != SLAP_C_USED ) return;
-
 	assert( c->c_conn_state != SLAP_C_INVALID );
 
 	/* c_mutex must be locked by caller */
@@ -830,8 +796,6 @@ connection_close( Connection *c )
 	assert( connections != NULL );
 	assert( c != NULL );
 
-	if ( c->c_struct_state != SLAP_C_USED ) return;
-
 	assert( c->c_conn_state == SLAP_C_CLOSING );
 
 	/* NOTE: c_mutex should be locked by caller */
@@ -881,13 +845,11 @@ Connection* connection_first( ber_socket_t *index )
 	assert( connections != NULL );
 	assert( index != NULL );
 
-	ldap_pvt_thread_mutex_lock( &connections_mutex );
 	for( *index = 0; *index < dtblsize; (*index)++) {
-		if( connections[*index].c_struct_state != SLAP_C_UNINITIALIZED ) {
+		if( connections[*index].c_sb ) {
 			break;
 		}
 	}
-	ldap_pvt_thread_mutex_unlock( &connections_mutex );
 
 	return connection_next(NULL, index);
 }
@@ -903,41 +865,19 @@ Connection* connection_next( Connection *c, ber_socket_t *index )
 
 	c = NULL;
 
-	ldap_pvt_thread_mutex_lock( &connections_mutex );
 	for(; *index < dtblsize; (*index)++) {
-		int c_struct;
-		if( connections[*index].c_struct_state == SLAP_C_UNINITIALIZED ) {
-			/* FIXME: accessing c_conn_state without locking c_mutex */
-			assert( connections[*index].c_conn_state == SLAP_C_INVALID );
-			continue;
-		}
-
-		if( connections[*index].c_struct_state == SLAP_C_USED ) {
+		if( connections[*index].c_sb ) {
 			c = &connections[(*index)++];
-			if ( ldap_pvt_thread_mutex_trylock( &c->c_mutex )) {
-				/* avoid deadlock */
-				ldap_pvt_thread_mutex_unlock( &connections_mutex );
-				ldap_pvt_thread_mutex_lock( &c->c_mutex );
-				ldap_pvt_thread_mutex_lock( &connections_mutex );
-			}
-			if ( c->c_struct_state != SLAP_C_USED ) {
+			ldap_pvt_thread_mutex_lock( &c->c_mutex );
+			if ( c->c_conn_state == SLAP_C_INVALID ) {
 				ldap_pvt_thread_mutex_unlock( &c->c_mutex );
 				c = NULL;
 				continue;
 			}
-			assert( c->c_conn_state != SLAP_C_INVALID );
 			break;
 		}
-
-		c_struct = connections[*index].c_struct_state;
-		if ( c_struct == SLAP_C_PENDING )
-			continue;
-		assert( c_struct == SLAP_C_UNUSED );
-		/* FIXME: accessing c_conn_state without locking c_mutex */
-		assert( connections[*index].c_conn_state == SLAP_C_INVALID );
 	}
 
-	ldap_pvt_thread_mutex_unlock( &connections_mutex );
 	return c;
 }
 
@@ -1290,7 +1230,6 @@ void connection_client_stop(
 		ber_sockbuf_ctrl( c->c_sb, LBER_SB_OPT_SET_MAX_INCOMING, &max );
 	}
 	c->c_conn_state = SLAP_C_INVALID;
-	c->c_struct_state = SLAP_C_UNUSED;
 	slapd_remove( s, sb, 0, 1, 0 );
 
 	connection_return( c );
@@ -1767,7 +1706,6 @@ connection_input( Connection *conn , conn_readinfo *cri )
 		}
 	}
 
-	assert( conn->c_struct_state == SLAP_C_USED );
 	return rc;
 }
 

servers/slapd/slap.h

@@ -2849,14 +2849,6 @@ typedef struct Listener Listener;
 /*
  * represents a connection from an ldap client
  */
-/* structure state (protected by connections_mutex) */
-enum sc_struct_state {
-	SLAP_C_UNINITIALIZED = 0,	/* MUST BE ZERO (0) */
-	SLAP_C_UNUSED,
-	SLAP_C_USED,
-	SLAP_C_PENDING
-};
-
 /* connection state (protected by c_mutex ) */
 enum sc_conn_state {
 	SLAP_C_INVALID = 0,		/* MUST BE ZERO (0) */
@@ -2867,7 +2859,6 @@ enum sc_conn_state {
 	SLAP_C_CLIENT			/* outbound client conn */
 };
 struct Connection {
-	enum sc_struct_state	c_struct_state; /* structure management state */
 	enum sc_conn_state	c_conn_state;	/* connection state */
 	int			c_conn_idx;		/* slot in connections array */
 	ber_socket_t	c_sd;

servers/slapd/slapi/slapi_ops.c

@@ -250,8 +250,7 @@ slapi_int_connection_init_pb( Slapi_PBlock *pb, ber_tag_t tag )
 	 */
 	connection_assign_nextid( conn );
 
-	conn->c_conn_state  = 0x01;	/* SLAP_C_ACTIVE */
-	conn->c_struct_state = 0x02;	/* SLAP_C_USED */
+	conn->c_conn_state  = SLAP_C_ACTIVE;
 
 	conn->c_ssf = conn->c_transport_ssf = local_ssf;
 	conn->c_tls_ssf = 0;