From 1a171b07d3776ee1643ea93adfd0c4f9230f8012 Mon Sep 17 00:00:00 2001 From: Howard Chu Date: Thu, 15 Jan 2009 20:41:40 +0000 Subject: [PATCH] ITS#5835 fix connection teradown when there are waiting writers --- servers/slapd/connection.c | 49 ++++++++++++++------------ servers/slapd/result.c | 61 +++++++++++++++++++++++++-------- servers/slapd/slap.h | 9 +++-- servers/slapd/slapi/slapi_ops.c | 6 ++-- 4 files changed, 83 insertions(+), 42 deletions(-) diff --git a/servers/slapd/connection.c b/servers/slapd/connection.c index 179025a9f0..f20a0552b4 100644 --- a/servers/slapd/connection.c +++ b/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 ); diff --git a/servers/slapd/result.c b/servers/slapd/result.c index a2e7343eb6..b5c6b90f5f 100644 --- a/servers/slapd/result.c +++ b/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 diff --git a/servers/slapd/slap.h b/servers/slapd/slap.h index dadd88d5c9..b4e50e3912 100644 --- a/servers/slapd/slap.h +++ b/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 diff --git a/servers/slapd/slapi/slapi_ops.c b/servers/slapd/slapi/slapi_ops.c index e7aab5d8d3..eaf4060b01 100644 --- a/servers/slapd/slapi/slapi_ops.c +++ b/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 ); -- 2.39.5