From da57548e5254c57f9a5b523c32a1ba7239b31673 Mon Sep 17 00:00:00 2001 From: Howard Chu Date: Mon, 22 Mar 2021 15:06:13 +0000 Subject: [PATCH] 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 | 106 +++++++------------------------- servers/slapd/slap.h | 9 --- servers/slapd/slapi/slapi_ops.c | 3 +- 3 files changed, 23 insertions(+), 95 deletions(-) diff --git a/servers/slapd/connection.c b/servers/slapd/connection.c index 53c801c813..5bd9fb831c 100644 --- a/servers/slapd/connection.c +++ b/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 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] ); - } + /* 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; } diff --git a/servers/slapd/slap.h b/servers/slapd/slap.h index 40e6aa6f98..ee0a7cd61f 100644 --- a/servers/slapd/slap.h +++ b/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; diff --git a/servers/slapd/slapi/slapi_ops.c b/servers/slapd/slapi/slapi_ops.c index 98ae8280c6..9bff32f308 100644 --- a/servers/slapd/slapi/slapi_ops.c +++ b/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;