[Date Prev][Date Next] [Chronological] [Thread] [Top]

RE: socket close race (ITS#2035)



Here is the suggested fix. It is untested, try at your own risk. (Diffs
relative to OPENLDAP_REL_ENG_1_2, corresponding to 1.2.13.)

--- connection.c	Tue Aug 31 15:33:54 1999
+++ connection.c.hyc1	Fri Aug 23 16:52:35 2002
@@ -130,6 +130,8 @@
 		return;
 	}

+	ldap_pvt_thread_rdwr_rlock( &conn->c_rdwr );
+
 	errno = 0;
 	if ( (tag = ber_get_next( &conn->c_sb, &len, conn->c_currentber ))
 	    != LDAP_TAG_MESSAGE ) {
@@ -147,6 +149,7 @@
 			ber_free( conn->c_currentber, 1 );
 			conn->c_currentber = NULL;

+			ldap_pvt_thread_rdwr_runlock( &conn->c_rdwr );
 			close_connection( conn, conn->c_connid, -1 );
 		}

@@ -161,6 +164,7 @@
 		    0 );
 		ber_free( ber, 1 );

+		ldap_pvt_thread_rdwr_runlock( &conn->c_rdwr );
 		close_connection( conn, conn->c_connid, -1 );
 		return;
 	}
@@ -171,6 +175,7 @@
 		    0 );
 		ber_free( ber, 1 );

+		ldap_pvt_thread_rdwr_runlock( &conn->c_rdwr );
 		close_connection( conn, conn->c_connid, -1 );
 		return;
 	}
@@ -196,6 +201,7 @@
 	arg->co_op = slap_op_add( &conn->c_ops, ber, msgid, tag, tmpdn,
 	    conn->c_opsinitiated, conn->c_connid );
 	ldap_pvt_thread_mutex_unlock( &conn->c_opsmutex );
+	ldap_pvt_thread_rdwr_runlock( &conn->c_rdwr );

 	if ( tmpdn != NULL ) {
 		free( tmpdn );
--- daemon.c	Sun May  6 17:03:35 2001
+++ daemon.c.hyc1	Fri Aug 23 17:01:27 2002
@@ -92,6 +92,7 @@
 		ldap_pvt_thread_mutex_init( &c[i].c_dnmutex );
 		ldap_pvt_thread_mutex_init( &c[i].c_opsmutex );
 		ldap_pvt_thread_mutex_init( &c[i].c_pdumutex );
+		ldap_pvt_thread_rdwr_init( &c[i].c_rdwr );
 		ldap_pvt_thread_cond_init( &c[i].c_wcv );
 	}

--- result.c	Fri Oct 29 11:00:47 1999
+++ result.c.hyc1	Fri Aug 23 17:06:27 2002
@@ -83,16 +83,17 @@
 		return;
 	}

+	ldap_pvt_thread_rdwr_rlock( &conn->c_rdwr );
+
 	/* write only one pdu at a time - wait til it's our turn */
 	ldap_pvt_thread_mutex_lock( &conn->c_pdumutex );

 	/* write the pdu */
 	bytes = ber->ber_ptr - ber->ber_buf;
-	ldap_pvt_thread_mutex_lock( &new_conn_mutex );
 	while ( conn->c_connid == op->o_connid && ber_flush( &conn->c_sb, ber,
 	    0 ) != 0 ) {
 		int err = errno;
-		ldap_pvt_thread_mutex_unlock( &new_conn_mutex );
+		ldap_pvt_thread_rdwr_runlock( &conn->c_rdwr );
 		/*
 		 * we got an error.  if it's ewouldblock, we need to
 		 * wait on the socket being writable.  otherwise, figure
@@ -126,21 +127,22 @@
 		ldap_pvt_thread_mutex_unlock( &active_threads_mutex );

 		ldap_pvt_thread_yield();
-		ldap_pvt_thread_mutex_lock( &new_conn_mutex );
+		ldap_pvt_thread_rdwr_rlock( &conn->c_rdwr );
 	}
-	ldap_pvt_thread_mutex_unlock( &new_conn_mutex );
 	ldap_pvt_thread_mutex_unlock( &conn->c_pdumutex );

 	ber_free( ber, 1 );

-	ldap_pvt_thread_mutex_lock( &num_sent_mutex );
-	num_bytes_sent += bytes;
-	ldap_pvt_thread_mutex_unlock( &num_sent_mutex );
-
 	Statslog( LDAP_DEBUG_STATS,
 	    "conn=%d op=%d RESULT err=%d tag=%d nentries=%d\n", conn->c_connid,
 	    op->o_opid, err, tag, nentries );

+	ldap_pvt_thread_rdwr_runlock( &conn->c_rdwr );
+
+	ldap_pvt_thread_mutex_lock( &num_sent_mutex );
+	num_bytes_sent += bytes;
+	ldap_pvt_thread_mutex_unlock( &num_sent_mutex );
+
 	return;
 }

@@ -315,15 +317,16 @@
 		return( 1 );
 	}

+	ldap_pvt_thread_rdwr_rlock( &conn->c_rdwr );
+
 	/* write only one pdu at a time - wait til it's our turn */
 	ldap_pvt_thread_mutex_lock( &conn->c_pdumutex );

 	bytes = ber->ber_ptr - ber->ber_buf;
-	ldap_pvt_thread_mutex_lock( &new_conn_mutex );
 	while ( conn->c_connid == op->o_connid && ber_flush( &conn->c_sb, ber,
 	    0 ) != 0 ) {
 		int err = errno;
-		ldap_pvt_thread_mutex_unlock( &new_conn_mutex );
+		ldap_pvt_thread_rdwr_runlock( &conn->c_rdwr );
 		/*
 		 * we got an error.  if it's ewouldblock, we need to
 		 * wait on the socket being writable.  otherwise, figure
@@ -355,19 +358,12 @@
 		ldap_pvt_thread_mutex_unlock( &active_threads_mutex );

 		ldap_pvt_thread_yield();
-		ldap_pvt_thread_mutex_lock( &new_conn_mutex );
+		ldap_pvt_thread_rdwr_rlock( &conn->c_rdwr );
 	}
-	ldap_pvt_thread_mutex_unlock( &new_conn_mutex );
 	ldap_pvt_thread_mutex_unlock( &conn->c_pdumutex );

 	ber_free( ber, 1 );

-	ldap_pvt_thread_mutex_lock( &num_sent_mutex );
-	num_bytes_sent += bytes;
-	num_entries_sent++;
-	ldap_pvt_thread_mutex_unlock( &num_sent_mutex );
-
-	ldap_pvt_thread_mutex_lock( &new_conn_mutex );
 	if ( conn->c_connid == op->o_connid ) {
 		rc = 0;
 		Statslog( LDAP_DEBUG_STATS2, "conn=%d op=%d ENTRY dn=\"%s\"\n",
@@ -375,7 +371,12 @@
 	} else {
 		rc = -1;
 	}
-	ldap_pvt_thread_mutex_unlock( &new_conn_mutex );
+	ldap_pvt_thread_rdwr_runlock( &conn->c_rdwr );
+
+	ldap_pvt_thread_mutex_lock( &num_sent_mutex );
+	num_bytes_sent += bytes;
+	num_entries_sent++;
+	ldap_pvt_thread_mutex_unlock( &num_sent_mutex );

 	Debug( LDAP_DEBUG_TRACE, "<= send_search_entry\n", 0, 0, 0 );

@@ -448,6 +449,7 @@
 void
 close_connection( Connection *conn, int opconnid, int opid )
 {
+	ldap_pvt_thread_rdwr_wlock( &conn->c_rdwr );
 	ldap_pvt_thread_mutex_lock( &new_conn_mutex );
 	if ( conn->c_sb.sb_sd != -1 && conn->c_connid == opconnid ) {
 		int err;
@@ -460,4 +462,5 @@
 		conn->c_version = 0;
 	}
 	ldap_pvt_thread_mutex_unlock( &new_conn_mutex );
+	ldap_pvt_thread_rdwr_wunlock( &conn->c_rdwr );
 }
--- slap.h	Fri Jul  9 12:06:57 1999
+++ slap.h.hyc1	Fri Aug 23 14:17:56 2002
@@ -319,6 +319,7 @@
 	Operation	*c_ops;		/* list of pending operations	  */
 	ldap_pvt_thread_mutex_t	c_opsmutex;	/* mutex for c_ops list & stats	  */
 	ldap_pvt_thread_mutex_t	c_pdumutex;	/* only one pdu written at a time */
+	ldap_pvt_thread_rdwr_t	c_rdwr;	/* reading or writing the conn */
 	ldap_pvt_thread_cond_t	c_wcv;		/* used to wait for sd write-ready*/
 	int		c_gettingber;	/* in the middle of ber_get_next  */
 	BerElement	*c_currentber;	/* ber we're getting              */


  -- Howard Chu
  Chief Architect, Symas Corp.       Director, Highland Sun
  http://www.symas.com               http://highlandsun.com/hyc
  Symas: Premier OpenSource Development and Support