ITS#5835 fix connection teradown when there are waiting writers

servers/slapd/connection.c

@@ -163,8 +163,10 @@ int connections_destroy(void)
 		if( connections[i].c_struct_state != SLAP_C_UNINITIALIZED ) {
 			ber_sockbuf_free( connections[i].c_sb );
 			ldap_pvt_thread_mutex_destroy( &connections[i].c_mutex );
-			ldap_pvt_thread_mutex_destroy( &connections[i].c_write_mutex );
-			ldap_pvt_thread_cond_destroy( &connections[i].c_write_cv );
+			ldap_pvt_thread_mutex_destroy( &connections[i].c_write1_mutex );
+			ldap_pvt_thread_mutex_destroy( &connections[i].c_write2_mutex );
+			ldap_pvt_thread_cond_destroy( &connections[i].c_write1_cv );
+			ldap_pvt_thread_cond_destroy( &connections[i].c_write2_cv );
 #ifdef LDAP_SLAPI
 			if ( slapi_plugins_used ) {
 				slapi_int_free_object_extensions( SLAPI_X_EXT_CONNECTION,
@@ -387,8 +389,10 @@ Connection * connection_init(
 
 		/* 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 );
+		ldap_pvt_thread_mutex_init( &c->c_write1_mutex );
+		ldap_pvt_thread_mutex_init( &c->c_write2_mutex );
+		ldap_pvt_thread_cond_init( &c->c_write1_cv );
+		ldap_pvt_thread_cond_init( &c->c_write2_cv );
 
 #ifdef LDAP_SLAPI
 		if ( slapi_plugins_used ) {
@@ -420,6 +424,7 @@ Connection * connection_init(
 	assert( c->c_sasl_bindop == NULL );
 	assert( c->c_currentber == NULL );
 	assert( c->c_writewaiter == 0);
+	assert( c->c_writers == 0);
 
 	c->c_listener = listener;
 	c->c_sd = s;
@@ -596,6 +601,7 @@ connection_destroy( Connection *c )
 	assert( LDAP_STAILQ_EMPTY(&c->c_txn_ops) );
 #endif
 	assert( c->c_writewaiter == 0);
+	assert( c->c_writers == 0);
 
 	/* only for stats (print -1 as "%lu" may give unexpected results ;) */
 	connid = c->c_connid;
@@ -755,17 +761,22 @@ void connection_closing( Connection *c, const char *why )
 		connection_abandon( c );
 
 		/* wake write blocked operations */
-		if ( c->c_writewaiter ) {
-			ldap_pvt_thread_cond_signal( &c->c_write_cv );
-			/* ITS#4667 this may allow another thread to drop into
-			 * connection_resched / connection_close before we
-			 * finish, but that's OK.
-			 */
-			slapd_clr_write( c->c_sd, 1 );
-			ldap_pvt_thread_mutex_unlock( &c->c_mutex );
-			ldap_pvt_thread_mutex_lock( &c->c_write_mutex );
-			ldap_pvt_thread_mutex_lock( &c->c_mutex );
-			ldap_pvt_thread_mutex_unlock( &c->c_write_mutex );
+		if ( c->c_writers > 0 ) {
+			ldap_pvt_thread_mutex_lock( &c->c_write1_mutex );
+			c->c_writers = -c->c_writers;
+			ldap_pvt_thread_cond_broadcast( &c->c_write1_cv );
+			ldap_pvt_thread_mutex_unlock( &c->c_write1_mutex );
+			if ( c->c_writewaiter ) {
+				ldap_pvt_thread_mutex_lock( &c->c_write2_mutex );
+				ldap_pvt_thread_cond_signal( &c->c_write2_cv );
+				slapd_clr_write( c->c_sd, 1 );
+				ldap_pvt_thread_mutex_unlock( &c->c_write2_mutex );
+			}
+			ldap_pvt_thread_mutex_lock( &c->c_write1_mutex );
+			while ( c->c_writers ) {
+				ldap_pvt_thread_cond_wait( &c->c_write1_cv, &c->c_write1_mutex );
+			}
+			ldap_pvt_thread_mutex_unlock( &c->c_write1_mutex );
 		} else {
 			slapd_clr_write( c->c_sd, 1 );
 		}
@@ -781,11 +792,6 @@ connection_close( Connection *c )
 {
 	assert( connections != NULL );
 	assert( c != NULL );
-
-	/* ITS#4667 we may have gotten here twice */
-	if ( c->c_conn_state == SLAP_C_INVALID )
-		return;
-
 	assert( c->c_struct_state == SLAP_C_USED );
 	assert( c->c_conn_state == SLAP_C_CLOSING );
 
@@ -1249,7 +1255,6 @@ int connection_read_activate( ber_socket_t s )
 	return rc;
 }
 
-/* Used for epoll / event functions that distinguish hangups from read events */
 void
 connection_hangup( ber_socket_t s )
 {
@@ -1848,7 +1853,7 @@ int connection_write(ber_socket_t s)
 	Debug( LDAP_DEBUG_TRACE,
 		"connection_write(%d): waking output for id=%lu\n",
 		s, c->c_connid, 0 );
-	ldap_pvt_thread_cond_signal( &c->c_write_cv );
+	ldap_pvt_thread_cond_signal( &c->c_write2_cv );
 
 	if ( ber_sockbuf_ctrl( c->c_sb, LBER_SB_OPT_NEEDS_READ, NULL ) ) {
 		slapd_set_read( s, 1 );

servers/slapd/result.c

@@ -137,27 +137,40 @@ static long send_ldap_ber(
 	BerElement *ber )
 {
 	ber_len_t bytes;
+	long ret = 0;
+	int closing = 0;
 
 	ber_get_option( ber, LBER_OPT_BER_BYTES_TO_WRITE, &bytes );
 
 	/* write only one pdu at a time - wait til it's our turn */
-	ldap_pvt_thread_mutex_lock( &conn->c_write_mutex );
+	ldap_pvt_thread_mutex_lock( &conn->c_write1_mutex );
+	while ( conn->c_writers > 0 ) {
+		ldap_pvt_thread_cond_wait( &conn->c_write1_cv, &conn->c_write1_mutex );
+	}
+	/* connection was closed under us */
+	if ( conn->c_writers < 0 ) {
+		closing = 1;
+		/* we're the last waiter, let the closer continue */
+		if ( conn->c_writers == -1 )
+			ldap_pvt_thread_cond_signal( &conn->c_write1_cv );
+	}
 
-	/* lock the connection */ 
-	ldap_pvt_thread_mutex_lock( &conn->c_mutex );
+	conn->c_writers++;
+	ldap_pvt_thread_mutex_unlock( &conn->c_write1_mutex );
+
+	if ( closing )
+		return 0;
 
 	/* write the pdu */
 	while( 1 ) {
 		int err;
 
-		if ( connection_state_closing( conn ) ) {
-			ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
-			ldap_pvt_thread_mutex_unlock( &conn->c_write_mutex );
-
-			return 0;
-		}
+		/* lock the connection */ 
+		ldap_pvt_thread_mutex_lock( &conn->c_mutex );
 
 		if ( ber_flush2( conn->c_sb, ber, LBER_FLUSH_FREE_NEVER ) == 0 ) {
+			ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
+			ret = bytes;
 			break;
 		}
 
@@ -176,23 +189,41 @@ static long send_ldap_ber(
 			connection_closing( conn, "connection lost on write" );
 
 			ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
-			ldap_pvt_thread_mutex_unlock( &conn->c_write_mutex );
 
-			return( -1 );
+			ret = -1;
+			break;
 		}
 
 		/* wait for socket to be write-ready */
+		ldap_pvt_thread_mutex_lock( &conn->c_write2_mutex );
 		conn->c_writewaiter = 1;
 		slapd_set_write( conn->c_sd, 1 );
 
-		ldap_pvt_thread_cond_wait( &conn->c_write_cv, &conn->c_mutex );
+		ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
+		ldap_pvt_thread_cond_wait( &conn->c_write2_cv, &conn->c_write2_mutex );
 		conn->c_writewaiter = 0;
+		ldap_pvt_thread_mutex_unlock( &conn->c_write2_mutex );
+		ldap_pvt_thread_mutex_lock( &conn->c_write1_mutex );
+		closing = ( conn->c_writers < 0 );
+		ldap_pvt_thread_mutex_unlock( &conn->c_write1_mutex );
+		if ( closing ) {
+			ret = 0;
+			break;
+		}
 	}
 
-	ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
-	ldap_pvt_thread_mutex_unlock( &conn->c_write_mutex );
+	ldap_pvt_thread_mutex_lock( &conn->c_write1_mutex );
+	if ( conn->c_writers < 0 ) {
+		conn->c_writers++;
+		if ( !conn->c_writers )
+			ldap_pvt_thread_cond_signal( &conn->c_write1_cv );
+	} else {
+		conn->c_writers--;
+		ldap_pvt_thread_cond_signal( &conn->c_write1_cv );
+	}
+	ldap_pvt_thread_mutex_unlock( &conn->c_write1_mutex );
 
-	return bytes;
+	return ret;
 }
 
 static int

servers/slapd/slap.h

@@ -2801,14 +2801,17 @@ struct Connection {
 	LDAP_STAILQ_HEAD(c_o, Operation) c_ops;	/* list of operations being processed */
 	LDAP_STAILQ_HEAD(c_po, Operation) c_pending_ops;	/* list of pending operations */
 
-	ldap_pvt_thread_mutex_t	c_write_mutex;	/* only one pdu written at a time */
-	ldap_pvt_thread_cond_t	c_write_cv;		/* used to wait for sd write-ready*/
+	ldap_pvt_thread_mutex_t	c_write1_mutex;	/* only one pdu written at a time */
+	ldap_pvt_thread_cond_t	c_write1_cv;	/* only one pdu written at a time */
+	ldap_pvt_thread_mutex_t	c_write2_mutex;	/* used to wait for sd write-ready */
+	ldap_pvt_thread_cond_t	c_write2_cv;	/* used to wait for sd write-ready*/
 
 	BerElement	*c_currentber;	/* ber we're attempting to read */
+	int			c_writers;		/* number of writers waiting */
 
 	char		c_sasl_bind_in_progress;	/* multi-op bind in progress */
+	char		c_writewaiter;	/* true if blocked on write */
 
-	char		c_writewaiter;	/* true if writer is waiting */
 
 #define	CONN_IS_TLS	1
 #define	CONN_IS_UDP	2

servers/slapd/slapi/slapi_ops.c

@@ -224,8 +224,10 @@ slapi_int_connection_init_pb( Slapi_PBlock *pb, ber_tag_t tag )
 
 	/* should check status of thread calls */
 	ldap_pvt_thread_mutex_init( &conn->c_mutex );
-	ldap_pvt_thread_mutex_init( &conn->c_write_mutex );
-	ldap_pvt_thread_cond_init( &conn->c_write_cv );
+	ldap_pvt_thread_mutex_init( &conn->c_write1_mutex );
+	ldap_pvt_thread_mutex_init( &conn->c_write2_mutex );
+	ldap_pvt_thread_cond_init( &conn->c_write1_cv );
+	ldap_pvt_thread_cond_init( &conn->c_write2_cv );
 
 	ldap_pvt_thread_mutex_lock( &conn->c_mutex );