From eca23a4002b69cd5146e5f5d7230c353f0ebb1b9 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Fri, 15 Jan 2016 11:35:16 -0500 Subject: [PATCH] Ticket 48412 - worker threads do not detect abnormally closed connections Bug Description: If a connection is abnormally closed there can still be data in the connection buffer(bytes vs offset). This prevents the connection from being removed from the connection table. The worker thread then goes into a loop trying to read this data on an already closed connection. If there are enough abnormally closed conenction eventually all the worker threads are stuck, and new connections are not accepted. Fix Description: When looking if there is more data in the buffer check if the connection was closed, and return 0 (no more data). Also did a little code cleanup. https://fedorahosted.org/389/ticket/48412 Reviewed by: ? --- ldap/servers/slapd/connection.c | 46 ++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c index c2cec85..718ba9e 100644 --- a/ldap/servers/slapd/connection.c +++ b/ldap/servers/slapd/connection.c @@ -1110,9 +1110,16 @@ connection_read_ldap_data(Connection *conn, PRInt32 *err) } static size_t -conn_buffered_data_avail_nolock(Connection *conn) +conn_buffered_data_avail_nolock(Connection *conn, int *conn_closed) { - return conn->c_private->c_buffer_bytes - conn->c_private->c_buffer_offset; + if ( (conn->c_sd == SLAPD_INVALID_SOCKET) || (conn->c_flags & CONN_FLAG_CLOSING) ) { + /* connection is closed - ignore the buffer */ + *conn_closed = 1; + return 0; + } else { + *conn_closed = 0; + return conn->c_private->c_buffer_bytes - conn->c_private->c_buffer_offset; + } } /* Upon returning from this function, we have either: @@ -1135,6 +1142,7 @@ int connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, i PRErrorCode err = 0; PRInt32 syserr = 0; size_t buffer_data_avail; + int conn_closed = 0; PR_EnterMonitor(conn->c_mutex); /* @@ -1150,7 +1158,7 @@ int connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, i *tag = LBER_DEFAULT; /* First check to see if we have buffered data from "before" */ - if ((buffer_data_avail = conn_buffered_data_avail_nolock(conn))) { + if ((buffer_data_avail = conn_buffered_data_avail_nolock(conn, &conn_closed))) { /* If so, use that data first */ if ( 0 != get_next_from_buffer( buffer + conn->c_private->c_buffer_offset, @@ -1165,7 +1173,7 @@ int connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, i while (*tag == LBER_DEFAULT) { int ioblocktimeout_waits = config_get_ioblocktimeout() / CONN_TURBO_TIMEOUT_INTERVAL; /* We should never get here with data remaining in the buffer */ - PR_ASSERT( !new_operation || 0 == conn_buffered_data_avail_nolock(conn) ); + PR_ASSERT( !new_operation || !conn_buffered_data_avail_nolock(conn, &conn_closed)); /* We make a non-blocking read call */ if (CONNECTION_BUFFER_OFF != conn->c_private->use_buffer) { ret = connection_read_ldap_data(conn,&err); @@ -1277,8 +1285,12 @@ int connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, i } } /* If there is remaining buffered data, set the flag to tell the caller */ - if (conn_buffered_data_avail_nolock(conn)) { + if (conn_buffered_data_avail_nolock(conn, &conn_closed)) { *remaining_data = 1; + } else if (conn_closed){ + /* connection closed */ + ret = CONN_DONE; + goto done; } if ( *tag != LDAP_TAG_MESSAGE ) { @@ -1529,7 +1541,7 @@ connection_threadmain() continue; case CONN_SHUTDOWN: LDAPDebug( LDAP_DEBUG_TRACE, - "op_thread received shutdown signal\n", 0, 0, 0 ); + "op_thread received shutdown signal\n", 0, 0, 0 ); g_decr_active_threadcnt(); return; case CONN_FOUND_WORK_TO_DO: @@ -1550,8 +1562,9 @@ connection_threadmain() Slapi_DN *anon_sdn = slapi_sdn_new_normdn_byref( anon_dn ); reslimit_update_from_dn( pb->pb_conn, anon_sdn ); slapi_sdn_free( &anon_sdn ); - if (slapi_reslimit_get_integer_limit(pb->pb_conn, pb->pb_conn->c_idletimeout_handle, - &idletimeout) + if (slapi_reslimit_get_integer_limit(pb->pb_conn, + pb->pb_conn->c_idletimeout_handle, + &idletimeout) == SLAPI_RESLIMIT_STATUS_SUCCESS) { pb->pb_conn->c_idletimeout = idletimeout; @@ -1589,7 +1602,7 @@ connection_threadmain() op = pb->pb_op; maxthreads = config_get_maxthreadsperconn(); more_data = 0; - ret = connection_read_operation(conn,op,&tag,&more_data); + ret = connection_read_operation(conn, op, &tag, &more_data); if ((ret == CONN_DONE) || (ret == CONN_TIMEDOUT)) { slapi_log_error(SLAPI_LOG_CONNS, "connection_threadmain", "conn %" NSPRIu64 " read not ready due to %d - thread_turbo_flag %d more_data %d " @@ -1622,7 +1635,8 @@ connection_threadmain() /* turn off turbo mode immediately if any pb waiting in global queue */ if (thread_turbo_flag && !WORK_Q_EMPTY) { thread_turbo_flag = 0; - LDAPDebug2Args(LDAP_DEBUG_CONNS,"conn %" NSPRIu64 " leaving turbo mode - pb_q is not empty %d\n",conn->c_connid,work_q_size); + LDAPDebug2Args(LDAP_DEBUG_CONNS,"conn %" NSPRIu64 " leaving turbo mode - pb_q is not empty %d\n", + conn->c_connid,work_q_size); } #endif @@ -1647,7 +1661,8 @@ connection_threadmain() * should call connection_make_readable after the op is removed * connection_make_readable(conn); */ - LDAPDebug(LDAP_DEBUG_CONNS,"conn %" NSPRIu64 " leaving turbo mode due to %d\n",conn->c_connid,ret,0); + LDAPDebug(LDAP_DEBUG_CONNS,"conn %" NSPRIu64 " leaving turbo mode due to %d\n", + conn->c_connid,ret,0); goto done; case CONN_SHUTDOWN: LDAPDebug( LDAP_DEBUG_TRACE, @@ -1703,7 +1718,8 @@ connection_threadmain() */ conn->c_idlesince = curtime; connection_activity(conn, maxthreads); - LDAPDebug(LDAP_DEBUG_CONNS,"conn %" NSPRIu64 " queued because more_data\n",conn->c_connid,0,0); + LDAPDebug(LDAP_DEBUG_CONNS,"conn %" NSPRIu64 " queued because more_data\n", + conn->c_connid,0,0); } else { /* keep count of how many times maxthreads has blocked an operation */ conn->c_maxthreadsblocked++; @@ -1778,13 +1794,15 @@ done: memset(pb, 0, sizeof(*pb)); } else { /* delete from connection operation queue & decr refcnt */ + int conn_closed = 0; PR_EnterMonitor(conn->c_mutex); connection_remove_operation_ext( pb, conn, op ); /* If we're in turbo mode, we keep our reference to the connection alive */ /* can't use the more_data var because connection could have changed in another thread */ - more_data = conn_buffered_data_avail_nolock(conn) ? 1 : 0; - LDAPDebug(LDAP_DEBUG_CONNS,"conn %" NSPRIu64 " check more_data %d thread_turbo_flag %d\n",conn->c_connid,more_data,thread_turbo_flag); + more_data = conn_buffered_data_avail_nolock(conn, &conn_closed) ? 1 : 0; + LDAPDebug(LDAP_DEBUG_CONNS,"conn %" NSPRIu64 " check more_data %d thread_turbo_flag %d\n", + conn->c_connid,more_data,thread_turbo_flag); if (!more_data) { if (!thread_turbo_flag) { /* -- 2.4.3