PROTOTYPE: New connection management infrastructure designed to

remove race conditions on connection close.
BROKEN: various counters for dn=monitor.
Initial testing on FreeBSD (with and without pthreads) was successfull.
Have not yet tested preemptive threading environments.
Have not built against backends other than LDBM.

servers/slapd/abandon.c

@@ -49,19 +49,28 @@ do_abandon(
 	 * flag and abort the operation at a convenient time.
 	 */
 
-	ldap_pvt_thread_mutex_lock( &conn->c_opsmutex );
+	ldap_pvt_thread_mutex_lock( &conn->c_mutex );
+
 	for ( o = conn->c_ops; o != NULL; o = o->o_next ) {
+		if ( o->o_msgid == id )
+			goto found_op;
+	}
+	for ( o = conn->c_pending_ops; o != NULL; o = o->o_next ) {
 		if ( o->o_msgid == id )
 			break;
 	}
 
+found_op:
+
 	if ( o != NULL ) {
 		ldap_pvt_thread_mutex_lock( &o->o_abandonmutex );
 		o->o_abandon = 1;
 		ldap_pvt_thread_mutex_unlock( &o->o_abandonmutex );
+
 	} else {
 		Debug( LDAP_DEBUG_TRACE, "do_abandon: op not found\n", 0, 0,
 		    0 );
 	}
-	ldap_pvt_thread_mutex_unlock( &conn->c_opsmutex );
+
+	ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
 }

servers/slapd/acl.c

@@ -287,7 +287,9 @@ acl_access_allowed(
 			}
 		}
 		if ( b->a_addrpat != NULL ) {
-			if ( regex_matches( b->a_addrpat, conn->c_addr, edn, matches ) ) {
+			if ( regex_matches( b->a_addrpat, conn->c_client_addr,
+				edn, matches ) )
+			{
 				Debug( LDAP_DEBUG_ACL,
 				    "<= acl_access_allowed: matched by clause #%d access %s\n",
 				    i, (b->a_access & ~ACL_SELF) >= access ?
@@ -299,7 +301,8 @@ acl_access_allowed(
 		if ( b->a_domainpat != NULL ) {
 			Debug( LDAP_DEBUG_ARGS, "<= check a_domainpath: %s\n",
 				b->a_domainpat, 0, 0 );
-			if ( regex_matches( b->a_domainpat, conn->c_domain, edn, matches ) ) 
+			if ( regex_matches( b->a_domainpat, conn->c_client_name,
+				edn, matches ) ) 
 			{
 				Debug( LDAP_DEBUG_ACL,
 				    "<= acl_access_allowed: matched by clause #%d access %s\n",

servers/slapd/add.c

@@ -151,6 +151,7 @@ add_created_attrs( Operation *op, Entry *e )
 	Attribute	**a, **next;
 	Attribute	*tmp;
 	struct tm	*ltm;
+	time_t		currenttime;
 
 	Debug( LDAP_DEBUG_TRACE, "add_created_attrs\n", 0, 0, 0 );
 
@@ -181,7 +182,8 @@ add_created_attrs( Operation *op, Entry *e )
 	}
 	attr_merge( e, "creatorsname", bvals );
 
-	ldap_pvt_thread_mutex_lock( &currenttime_mutex );
+	currenttime = slap_get_time();
+	ldap_pvt_thread_mutex_lock( &gmtime_mutex );
 #ifndef LDAP_LOCALTIME
 	ltm = gmtime( &currenttime );
 	strftime( buf, sizeof(buf), "%Y%m%d%H%M%SZ", ltm );
@@ -189,7 +191,7 @@ add_created_attrs( Operation *op, Entry *e )
 	ltm = localtime( &currenttime );
 	strftime( buf, sizeof(buf), "%y%m%d%H%M%SZ", ltm );
 #endif
-	ldap_pvt_thread_mutex_unlock( &currenttime_mutex );
+	ldap_pvt_thread_mutex_unlock( &gmtime_mutex );
 
 	bv.bv_val = buf;
 	bv.bv_len = strlen( bv.bv_val );

servers/slapd/back-ldbm/dbcache.c

@@ -41,9 +41,7 @@ ldbm_cache_open(
 	    flags, li->li_mode );
 
 	lru = 0;
-	ldap_pvt_thread_mutex_lock( &currenttime_mutex );
-	curtime = currenttime;
-	ldap_pvt_thread_mutex_unlock( &currenttime_mutex );
+	curtime = slap_get_time();
 	oldtime = curtime;
 
 	ldap_pvt_thread_mutex_lock( &li->li_dbcache_mutex );

servers/slapd/back-ldbm/search.c

@@ -139,6 +139,7 @@ ldbm_back_search(
 	rcur = strchr( rbuf, '\0' );
 	for ( id = idl_firstid( candidates ); id != NOID;
 	    id = idl_nextid( candidates, id ) ) {
+
 		/* check for abandon */
 		ldap_pvt_thread_mutex_lock( &op->o_abandonmutex );
 		if ( op->o_abandon ) {
@@ -153,10 +154,7 @@ ldbm_back_search(
 		ldap_pvt_thread_mutex_unlock( &op->o_abandonmutex );
 
 		/* check time limit */
-		ldap_pvt_thread_mutex_lock( &currenttime_mutex );
-		time( &currenttime );
-		if ( tlimit != -1 && currenttime > stoptime ) {
-			ldap_pvt_thread_mutex_unlock( &currenttime_mutex );
+		if ( tlimit != -1 && slap_get_time() > stoptime ) {
 			send_ldap_search_result( conn, op,
 			    LDAP_TIMELIMIT_EXCEEDED, NULL, nrefs > 0 ? rbuf :
 			    NULL, nentries );
@@ -167,7 +165,6 @@ ldbm_back_search(
 			}
 			return( 0 );
 		}
-		ldap_pvt_thread_mutex_unlock( &currenttime_mutex );
 
 		/* get the entry with reader lock */
 		if ( (e = id2entry_r( be, id )) == NULL ) {

servers/slapd/bind.c

@@ -135,7 +135,7 @@ do_bind(
 			free( cred.bv_val );
 		}
 
-		ldap_pvt_thread_mutex_lock( &conn->c_dnmutex );
+		ldap_pvt_thread_mutex_lock( &conn->c_mutex );
 
 		conn->c_protocol = version;
 
@@ -149,7 +149,7 @@ do_bind(
 			conn->c_dn = NULL;
 		}
 
-		ldap_pvt_thread_mutex_unlock( &conn->c_dnmutex );
+		ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
 
 		send_ldap_result( conn, op, LDAP_SUCCESS, NULL, NULL );
 		return;
@@ -168,7 +168,7 @@ do_bind(
 			free( cred.bv_val );
 		}
 		if ( cred.bv_len == 0 ) {
-			ldap_pvt_thread_mutex_lock( &conn->c_dnmutex );
+			ldap_pvt_thread_mutex_lock( &conn->c_mutex );
 
 			conn->c_protocol = version;
 
@@ -182,7 +182,7 @@ do_bind(
 				conn->c_dn = NULL;
 			}
 
-			ldap_pvt_thread_mutex_unlock( &conn->c_dnmutex );
+			ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
 
 			send_ldap_result( conn, op, LDAP_SUCCESS,
 				NULL, NULL );
@@ -203,7 +203,7 @@ do_bind(
 		ndn = suffixAlias( ndn, op, be );
 
 		if ( (*be->be_bind)( be, conn, op, ndn, method, &cred, &edn ) == 0 ) {
-			ldap_pvt_thread_mutex_lock( &conn->c_dnmutex );
+			ldap_pvt_thread_mutex_lock( &conn->c_mutex );
 
 			conn->c_protocol = version;
 
@@ -228,7 +228,7 @@ do_bind(
 			Debug( LDAP_DEBUG_TRACE, "do_bind: bound \"%s\" to \"%s\"\n",
 	    		conn->c_cdn, conn->c_dn, method );
 
-			ldap_pvt_thread_mutex_unlock( &conn->c_dnmutex );
+			ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
 
 			/* send this here to avoid a race condition */
 			send_ldap_result( conn, op, LDAP_SUCCESS, NULL, NULL );

servers/slapd/connection.c

@@ -10,6 +10,16 @@
 
 #include "slap.h"
 
+/* protected by connections_mutex */
+static ldap_pvt_thread_mutex_t connections_mutex;
+static Connection *connections = NULL;
+static int conn_index = -1;
+static long conn_nextid = 0;
+
+static Connection* connection_get( int s );
+
+static int connection_input( Connection *c );
+
 static int connection_op_activate( Connection *conn, Operation *op );
 static int connection_resched( Connection *conn );
 
@@ -18,6 +28,360 @@ struct co_arg {
 	Operation	*co_op;
 };
 
+/*
+ * Initialize connection management infrastructure.
+ */
+int connections_init(void)
+{
+	int i;
+
+	assert( connections == NULL );
+
+	if( connections != NULL) { /* probably should assert this */
+		Debug( LDAP_DEBUG_ANY, "connections_init: already initialized.\n",
+			0, 0, 0 );
+		return -1;
+	}
+
+	/* should check return of every call */
+	ldap_pvt_thread_mutex_init( &connections_mutex );
+
+	connections = (Connection *) calloc( dtblsize, sizeof(Connection) );
+
+	if( connections == NULL ) {
+		Debug( LDAP_DEBUG_ANY,
+			"connections_init: allocation (%d*%ld) of connection array failed.\n",
+			dtblsize, (long) sizeof(Connection), 0 );
+		return -1;
+	}
+
+	/*
+	 * per entry initialization of the Connection array initialization
+	 * will be done by connection_init()
+	 */ 
+
+	return 0;
+}
+
+static Connection* connection_get( int s )
+{
+	Connection *c = NULL;
+
+	assert( connections != NULL );
+
+	if(s < 0) {
+		return NULL;
+	}
+
+#ifndef HAVE_WINSOCK
+	assert( connections[s].c_struct_state == SLAP_C_USED );
+	assert( connections[s].c_conn_state != SLAP_C_INVALID );
+	assert( connections[s].c_sb.sb_sd != -1 );
+
+	c = &connections[s];
+#else
+	{
+		int i;
+
+		for(i=0; i<dtblsize; i++) {
+			if( connections[i].c_struct_state == SLAP_C_STRUCT_UNINITIALIZED ) {
+				assert( connections[i].c_conn_state == SLAP_C_INVALID );
+				assert( connections[s].c_sb.sb_sd == 0 );
+				break;
+			}
+
+			if( connections[i].c_struct_state == SLAP_C_STRUCT_UNUSED ) {
+				assert( connections[i].c_conn_state == SLAP_C_INVALID );
+				assert( connections[s].c_sb.sb_sd == -1 );
+				continue;
+			}
+
+			assert( connections[i].c_struct_state == SLAP_C_STRUCT_USED );
+			assert( connections[i].c_conn_state != SLAP_C_INVALID );
+			assert( connections[s].c_sb.sb_sd != -1 );
+
+			if( connections[i].c_sb.sb_sd == s ) {
+				c = &connections[s];
+				break;
+			}
+		}
+	}
+#endif
+	if( c != NULL ) {
+		ldap_pvt_thread_mutex_lock( &c->c_mutex );
+	}
+	return c;
+}
+
+static void connection_return( Connection *c )
+{
+	ldap_pvt_thread_mutex_unlock( &c->c_mutex );
+}
+
+long connection_init(
+	int s,
+	const char* name,
+	const char* addr)
+{
+	long id;
+	Connection *c;
+	assert( connections != NULL );
+
+	if( s < 0 ) {
+		return -1;
+	}
+
+	assert( s >= 0 );
+#ifndef HAVE_WINSOCK
+	assert( s < dtblsize );
+#endif
+
+	ldap_pvt_thread_mutex_lock( &connections_mutex );
+
+#ifndef HAVE_WINSOCK
+	c = &connections[s];
+
+#else
+	{
+		int i;
+
+		for( i=0; i < dtblsize; i++ {
+			if( connections[i].c_struct_state == SLAP_C_UNINITIALIZED ) {
+				assert( connections[i].c_sb.sb_sd == 0 );
+				c = &connections[i];
+				break;
+			}
+
+			if( connections[i].c_struct_state == SLAP_C_UNUSED ) {
+				assert( connections[i].c_sb.sb_sd == -1 );
+				c = &connections[i];
+				break;
+			}
+
+			assert( connections[i].c_struct_state == SLAP_C_USED );
+			assert( connections[i].c_conn_state != SLAP_C_INVALID );
+			assert( connections[i].c_sb.sb_sd != -1 );
+		}
+
+		if( c == NULL ) {
+			ldap_pvt_thread_mutex_unlock( &connections_mutex );
+			return -1;
+		}
+	}
+#endif
+
+	assert( c != NULL );
+	assert( c->c_struct_state != SLAP_C_USED );
+	assert( c->c_conn_state == SLAP_C_INVALID );
+
+	if( c->c_struct_state == SLAP_C_UNINITIALIZED ) {
+		c->c_dn = NULL;
+		c->c_cdn = NULL;
+		c->c_client_name = NULL;
+		c->c_client_addr = NULL;
+		c->c_ops = NULL;
+		c->c_pending_ops = NULL;
+
+		lber_pvt_sb_init( &c->c_sb );
+
+		/* should check status of thread calls */
+		ldap_pvt_thread_mutex_init( &c->c_mutex );
+		ldap_pvt_thread_mutex_init( &c->c_write_mutex );
+		ldap_pvt_thread_cond_init( &c->c_write_cv );
+
+		c->c_struct_state = SLAP_C_UNUSED;
+	}
+
+	ldap_pvt_thread_mutex_lock( &c->c_mutex );
+
+	assert( c->c_struct_state == SLAP_C_UNUSED );
+	assert(	c->c_dn == NULL );
+	assert(	c->c_cdn == NULL );
+	assert( c->c_client_name == NULL );
+	assert( c->c_client_addr == NULL );
+	assert( c->c_ops == NULL );
+	assert( c->c_pending_ops == NULL );
+
+	c->c_client_name = ch_strdup( name == NULL ? "" : name );
+	c->c_client_addr = ch_strdup( addr );
+
+	c->c_n_ops_received = 0;
+#ifdef LDAP_COUNTERS
+	c->c_n_ops_executing = 0;
+	c->c_n_ops_pending = 0;
+	c->c_n_ops_completed = 0;
+#endif
+
+	c->c_starttime = slap_get_time();
+
+	lber_pvt_sb_set_desc( &c->c_sb, s );
+	lber_pvt_sb_set_io( &c->c_sb, &lber_pvt_sb_io_tcp, NULL );
+
+	if( lber_pvt_sb_set_nonblock( &c->c_sb, 1 ) < 0 ) {
+		Debug( LDAP_DEBUG_ANY,
+			"connection_init(%d, %s, %s): set nonblocking failed\n",
+			s, c->c_client_name, c->c_client_addr);
+	}
+
+	id = c->c_connid = conn_nextid++;
+
+	c->c_conn_state = SLAP_C_INACTIVE;
+	c->c_struct_state = SLAP_C_USED;
+
+	ldap_pvt_thread_mutex_unlock( &c->c_mutex );
+	ldap_pvt_thread_mutex_unlock( &connections_mutex );
+
+	return id;
+}
+
+static void
+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( c->c_ops == NULL );
+
+	c->c_struct_state = SLAP_C_UNUSED;
+	c->c_conn_state = SLAP_C_INVALID;
+
+	c->c_version = 0;
+	c->c_protocol = 0;
+
+	c->c_starttime = 0;
+
+	if(c->c_dn != NULL) {
+		free(c->c_dn);
+		c->c_dn = NULL;
+	}
+	if(c->c_cdn != NULL) {
+		free(c->c_cdn);
+		c->c_cdn = NULL;
+	}
+	if(c->c_client_name != NULL) {
+		free(c->c_client_name);
+		c->c_client_name = NULL;
+	}
+	if(c->c_client_addr != NULL) {
+		free(c->c_client_addr);
+		c->c_client_addr = NULL;
+	}
+
+	if ( lber_pvt_sb_in_use(&c->c_sb) ) {
+		int sd = lber_pvt_sb_get_desc(&c->c_sb);
+
+		slapd_remove( sd );
+	   	lber_pvt_sb_close( &c->c_sb );
+
+		Statslog( LDAP_DEBUG_STATS,
+		    "conn=%d fd=%d closed.\n",
+			c->c_connid, sd, 0, 0, 0 );
+	}
+
+   	lber_pvt_sb_destroy( &c->c_sb );
+}
+
+static void connection_close( Connection *c )
+{
+	assert( connections != NULL );
+	assert( c != NULL );
+	assert( c->c_struct_state == SLAP_C_USED );
+	assert( c->c_conn_state == SLAP_C_CLOSING );
+
+	if( c->c_ops != NULL ) {
+		Debug( LDAP_DEBUG_TRACE,
+			"connection_close: deferring conn=%ld sd=%d.\n",
+			c->c_connid, c->c_sb.sb_sd, 0 );
+
+		return;
+	}
+
+	Debug( LDAP_DEBUG_TRACE, "connection_close: conn=%ld sd=%d.\n",
+		c->c_connid, c->c_sb.sb_sd, 0 );
+
+	connection_destroy( c );
+}
+
+long connections_nextid(void)
+{
+	long id;
+	assert( connections != NULL );
+
+	ldap_pvt_thread_mutex_lock( &connections_mutex );
+
+	id = conn_nextid;
+
+	ldap_pvt_thread_mutex_unlock( &connections_mutex );
+
+	return id;
+}
+
+Connection* connection_first(void)
+{
+	assert( connections != NULL );
+
+	ldap_pvt_thread_mutex_lock( &connections_mutex );
+
+	assert( conn_index == -1 );
+	conn_index = 0;
+
+	return connection_next(NULL);
+}
+
+Connection* connection_next(Connection *c)
+{
+	assert( connections != NULL );
+	assert( conn_index != -1 );
+	assert( conn_index <= dtblsize );
+
+	if( c != NULL ) {
+		ldap_pvt_thread_mutex_unlock( &c->c_mutex );
+	}
+
+	c = NULL;
+
+	for(; conn_index < dtblsize; conn_index++) {
+		if( connections[conn_index].c_struct_state == SLAP_C_UNINITIALIZED ) {
+			assert( connections[conn_index].c_conn_state == SLAP_C_INVALID );
+#ifndef HAVE_WINSOCK
+			continue;
+#else
+			break;
+#endif
+		}
+
+		if( connections[conn_index].c_struct_state == SLAP_C_USED ) {
+			assert( connections[conn_index].c_conn_state != SLAP_C_INVALID );
+			c = &connections[conn_index++];
+			break;
+		}
+
+		assert( connections[conn_index].c_struct_state == SLAP_C_UNUSED );
+		assert( connections[conn_index].c_conn_state == SLAP_C_INVALID );
+	}
+
+	if( c != NULL ) {
+		ldap_pvt_thread_mutex_lock( &c->c_mutex );
+	}
+
+	return c;
+}
+
+void connection_done(Connection *c)
+{
+	assert( connections != NULL );
+	assert( conn_index != -1 );
+	assert( conn_index <= dtblsize );
+
+	if( c != NULL ) {
+		ldap_pvt_thread_mutex_unlock( &c->c_mutex );
+	}
+
+	conn_index = -1;
+	ldap_pvt_thread_mutex_unlock( &connections_mutex );
+}
+
 /*
  * connection_activity - handle the request operation op on connection
  * conn.  This routine figures out what kind of operation it is and
@@ -31,13 +395,11 @@ connection_operation( void *arg_v )
 	int tag = arg->co_op->o_tag;
 	Connection *conn = arg->co_conn;
 
-	ldap_pvt_thread_mutex_lock( &conn->c_opsmutex );
-	conn->c_ops_received++;
-	ldap_pvt_thread_mutex_unlock( &conn->c_opsmutex );
-
+#ifdef LDAP_COUNTERS
 	ldap_pvt_thread_mutex_lock( &ops_mutex );
 	ops_initiated++;
 	ldap_pvt_thread_mutex_unlock( &ops_mutex );
+#endif
 
 	switch ( tag ) {
 	case LDAP_REQ_BIND:
@@ -91,12 +453,17 @@ connection_operation( void *arg_v )
 		break;
 	}
 
+#ifdef LDAP_COUNTERS
 	ldap_pvt_thread_mutex_lock( &ops_mutex );
 	ops_completed++;
 	ldap_pvt_thread_mutex_unlock( &ops_mutex );
+#endif
+
+	ldap_pvt_thread_mutex_lock( &conn->c_mutex );
 
-	ldap_pvt_thread_mutex_lock( &conn->c_opsmutex );
+#ifdef LDAP_COUNTERS
 	conn->c_ops_completed++;
+#endif
 
 	slap_op_remove( &conn->c_ops, arg->co_op );
 	slap_op_free( arg->co_op );
@@ -105,11 +472,27 @@ connection_operation( void *arg_v )
 	free( (char *) arg );
 	arg = NULL;
 
-	if((tag == LDAP_REQ_BIND) && (conn->c_state == SLAP_C_BINDING)) {
-		conn->c_state = SLAP_C_ACTIVE;
+	switch( tag ) {
+#ifdef LDAP_COMPAT30
+	case LDAP_REQ_UNBIND_30:
+#endif
+	case LDAP_REQ_UNBIND:
+		conn->c_conn_state = SLAP_C_CLOSING;
+		break;
+
+	case LDAP_REQ_BIND:
+		if( conn->c_conn_state == SLAP_C_BINDING) {
+			conn->c_conn_state = SLAP_C_ACTIVE;
+		}
 	}
 
-	ldap_pvt_thread_mutex_unlock( &conn->c_opsmutex );
+	if( conn->c_conn_state == SLAP_C_CLOSING ) {
+		Debug( LDAP_DEBUG_TRACE,
+			"connection_operation: attempting closing conn=%ld sd=%d.\n",
+			conn->c_connid, conn->c_sb.sb_sd, 0 );
+
+		connection_close( conn );
+	}
 
 	ldap_pvt_thread_mutex_lock( &active_threads_mutex );
 	active_threads--;
@@ -120,11 +503,58 @@ connection_operation( void *arg_v )
 
 	connection_resched( conn );