Improved "closing" handling. Remove fd from read set when state

changes to closing.  Need to add protection against read set races.

diff --git a/servers/slapd/connection.c b/servers/slapd/connection.c
index eb459637ce4311c0d1bf28dac4332036556f190d..5f88d1dc338f63e491addef56a3e7b48f5607418 100644 (file)
--- a/servers/slapd/connection.c
+++ b/servers/slapd/connection.c
@@ -16,6 +16,18 @@ static Connection *connections = NULL;
 static int conn_index = -1;
 static long conn_nextid = 0;
 
 static int conn_index = -1;
 static long conn_nextid = 0;
 

+/* structure state (protected by connections_mutex) */
+#define SLAP_C_UNINITIALIZED   0x0     /* MUST BE ZERO (0) */
+#define SLAP_C_UNUSED                  0x1
+#define SLAP_C_USED                            0x2
+
+/* connection state (protected by c_mutex ) */
+#define SLAP_C_INVALID                 0x0     /* MUST BE ZERO (0) */
+#define SLAP_C_INACTIVE                        0x1     /* zero threads */
+#define SLAP_C_ACTIVE                  0x2 /* one or more threads */
+#define SLAP_C_BINDING                 0x3     /* binding */
+#define SLAP_C_CLOSING                 0x4     /* closing */
+

 static Connection* connection_get( int s );
 
 static int connection_input( Connection *c );
 static Connection* connection_get( int s );
 
 static int connection_input( Connection *c );


@@ -288,6 +300,20 @@ connection_destroy( Connection *c )
        lber_pvt_sb_destroy( &c->c_sb );
 }
 
        lber_pvt_sb_destroy( &c->c_sb );
 }
 

+void connection_closing( Connection *c )
+{
+       assert( connections != NULL );
+       assert( c != NULL );
+       assert( c->c_struct_state == SLAP_C_USED );
+       assert( c->c_conn_state != SLAP_C_INVALID );
+
+       if( c->c_conn_state != SLAP_C_CLOSING ) {
+               /* don't listen on this port anymore */
+               slapd_clr_read( c->c_sb.sb_sd, 1 );
+               c->c_conn_state = SLAP_C_CLOSING;
+       }
+}
+

 static void connection_close( Connection *c )
 {
        assert( connections != NULL );
 static void connection_close( Connection *c )
 {
        assert( connections != NULL );


@@ -483,7 +509,7 @@ connection_operation( void *arg_v )
        case LDAP_REQ_UNBIND_30:
 #endif
        case LDAP_REQ_UNBIND:
        case LDAP_REQ_UNBIND_30:
 #endif
        case LDAP_REQ_UNBIND:

-               conn->c_conn_state = SLAP_C_CLOSING;
+               connection_closing( conn );

                break;
 
        case LDAP_REQ_BIND:
                break;
 
        case LDAP_REQ_BIND:


@@ -550,7 +576,7 @@ int connection_read(int s)
                        "connection_read(%d): input error id=%ld, closing.\n",
                        s, c->c_connid, 0 );
 
                        "connection_read(%d): input error id=%ld, closing.\n",
                        s, c->c_connid, 0 );
 

-               c->c_conn_state = SLAP_C_CLOSING;
+               connection_closing( c );

                connection_close( c );
        }
 
                connection_close( c );
        }
 


@@ -583,7 +609,8 @@ connection_input(
                        "ber_get_next on fd %d failed errno %d (%s)\n",
                        lber_pvt_sb_get_desc(&conn->c_sb), errno,
                        errno > -1 && errno < sys_nerr ?  sys_errlist[errno] : "unknown" );
                        "ber_get_next on fd %d failed errno %d (%s)\n",
                        lber_pvt_sb_get_desc(&conn->c_sb), errno,
                        errno > -1 && errno < sys_nerr ?  sys_errlist[errno] : "unknown" );

-               Debug( LDAP_DEBUG_TRACE, "\t*** got %ld of %lu so far\n",
+               Debug( LDAP_DEBUG_TRACE,
+                       "\t*** got %ld of %lu so far\n",

                        (long)(conn->c_currentber->ber_rwptr - conn->c_currentber->ber_buf),
                        conn->c_currentber->ber_len, 0 );
 
                        (long)(conn->c_currentber->ber_rwptr - conn->c_currentber->ber_buf),
                        conn->c_currentber->ber_len, 0 );
 

diff --git a/servers/slapd/daemon.c b/servers/slapd/daemon.c
index 72ece692e9911fe775b82cc1ad992c52b6428775..58954d3026f187619f4a90f1e2b8d7dcbaa5ab93 100644 (file)
--- a/servers/slapd/daemon.c
+++ b/servers/slapd/daemon.c
@@ -58,9 +58,11 @@ static void slapd_add(int s) {
        assert( !FD_ISSET( s, &slap_daemon.sd_readers ));
        assert( !FD_ISSET( s, &slap_daemon.sd_writers ));
 
        assert( !FD_ISSET( s, &slap_daemon.sd_readers ));
        assert( !FD_ISSET( s, &slap_daemon.sd_writers ));
 

+#ifndef HAVE_WINSOCK

        if (s >= slap_daemon.sd_nfds) {
                slap_daemon.sd_nfds = s + 1;
        }
        if (s >= slap_daemon.sd_nfds) {
                slap_daemon.sd_nfds = s + 1;
        }

+#endif

 
        FD_SET( s, &slap_daemon.sd_actives );
        FD_SET( s, &slap_daemon.sd_readers );
 
        FD_SET( s, &slap_daemon.sd_actives );
        FD_SET( s, &slap_daemon.sd_readers );


@@ -78,7 +80,6 @@ static void slapd_add(int s) {
 void slapd_remove(int s) {
        ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
 
 void slapd_remove(int s) {
        ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
 

-       assert( s < slap_daemon.sd_nfds );

        assert( FD_ISSET( s, &slap_daemon.sd_actives ));
 
        Debug( LDAP_DEBUG_CONNS, "daemon: removing %d%s%s\n", s,
        assert( FD_ISSET( s, &slap_daemon.sd_actives ));
 
        Debug( LDAP_DEBUG_CONNS, "daemon: removing %d%s%s\n", s,


@@ -95,10 +96,8 @@ void slapd_remove(int s) {
 void slapd_clr_write(int s, int wake) {
        ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
 
 void slapd_clr_write(int s, int wake) {
        ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
 

-       assert( s < slap_daemon.sd_nfds );

        assert( FD_ISSET( s, &slap_daemon.sd_actives) );
        assert( FD_ISSET( s, &slap_daemon.sd_actives) );

-       assert( FD_ISSET( s, &slap_daemon.sd_writers) );
-       FD_SET( s, &slap_daemon.sd_writers );
+       FD_CLR( s, &slap_daemon.sd_writers );

 
        ldap_pvt_thread_mutex_unlock( &slap_daemon.sd_mutex );
 
 
        ldap_pvt_thread_mutex_unlock( &slap_daemon.sd_mutex );
 


@@ -110,6 +109,7 @@ void slapd_clr_write(int s, int wake) {
 void slapd_set_write(int s, int wake) {
        ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
 
 void slapd_set_write(int s, int wake) {
        ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
 

+       assert( FD_ISSET( s, &slap_daemon.sd_actives) );

        FD_SET( s, &slap_daemon.sd_writers );
 
        ldap_pvt_thread_mutex_unlock( &slap_daemon.sd_mutex );
        FD_SET( s, &slap_daemon.sd_writers );
 
        ldap_pvt_thread_mutex_unlock( &slap_daemon.sd_mutex );


@@ -119,6 +119,32 @@ void slapd_set_write(int s, int wake) {
        }
 }
 
        }
 }
 

+void slapd_clr_read(int s, int wake) {
+       ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
+
+       assert( FD_ISSET( s, &slap_daemon.sd_actives) );
+       FD_CLR( s, &slap_daemon.sd_readers );
+
+       ldap_pvt_thread_mutex_unlock( &slap_daemon.sd_mutex );
+
+       if( wake ) {
+               ldap_pvt_thread_kill( listener_tid, LDAP_SIGUSR1 );
+       }
+}
+
+void slapd_set_read(int s, int wake) {
+       ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
+
+       assert( FD_ISSET( s, &slap_daemon.sd_actives) );
+       FD_SET( s, &slap_daemon.sd_readers );
+
+       ldap_pvt_thread_mutex_unlock( &slap_daemon.sd_mutex );
+
+       if( wake ) {
+               ldap_pvt_thread_kill( listener_tid, LDAP_SIGUSR1 );
+       }
+}
+

 static void slapd_close(int s) {
        slapd_remove(s);
 
 static void slapd_close(int s) {
        slapd_remove(s);
 

diff --git a/servers/slapd/proto-slap.h b/servers/slapd/proto-slap.h
index b4c90c0ea11cf742f2decf1d993cc0e06209bde1..e513159139c472fb1fd492b2b67a45de933e43e5 100644 (file)
--- a/servers/slapd/proto-slap.h
+++ b/servers/slapd/proto-slap.h
@@ -115,6 +115,8 @@ long connection_init LDAP_P((
        int s,
        const char* name, const char* addr));
 
        int s,
        const char* name, const char* addr));
 

+void connection_closing LDAP_P(( Connection *c ));
+

 int connection_write LDAP_P((int s));
 int connection_read LDAP_P((int s));
 
 int connection_write LDAP_P((int s));
 int connection_read LDAP_P((int s));
 


@@ -310,6 +312,11 @@ extern int slap_destroy LDAP_P((void));
 struct sockaddr_in;
 extern int slapd_daemon LDAP_P((struct sockaddr_in *addr));
 
 struct sockaddr_in;
 extern int slapd_daemon LDAP_P((struct sockaddr_in *addr));
 

+extern void slapd_set_write LDAP_P((int s, int wake));
+extern void slapd_clr_write LDAP_P((int s, int wake));
+extern void slapd_set_read LDAP_P((int s, int wake));
+extern void slapd_clr_read LDAP_P((int s, int wake));
+

 extern void    slap_set_shutdown LDAP_P((int sig));
 extern void    slap_do_nothing   LDAP_P((int sig));
 
 extern void    slap_set_shutdown LDAP_P((int sig));
 extern void    slap_do_nothing   LDAP_P((int sig));
 

diff --git a/servers/slapd/result.c b/servers/slapd/result.c
index c79d6de627070a2d3993d029d96b9a3992a40b10..2a041bf66b55ad4201e8bd1d766580a7363826d0 100644 (file)
--- a/servers/slapd/result.c
+++ b/servers/slapd/result.c
@@ -104,7 +104,7 @@ send_ldap_result2(
                    : "unknown", 0 );
 
                if ( errno != EWOULDBLOCK && errno != EAGAIN ) {
                    : "unknown", 0 );
 
                if ( errno != EWOULDBLOCK && errno != EAGAIN ) {

-                       conn->c_conn_state = SLAP_C_CLOSING;
+                       connection_closing( conn );

 
                        ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
                        ldap_pvt_thread_mutex_unlock( &conn->c_write_mutex );
 
                        ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
                        ldap_pvt_thread_mutex_unlock( &conn->c_write_mutex );


@@ -334,7 +334,7 @@ send_search_entry(
                    : "unknown", 0 );
 
                if ( errno != EWOULDBLOCK && errno != EAGAIN ) {
                    : "unknown", 0 );
 
                if ( errno != EWOULDBLOCK && errno != EAGAIN ) {

-                       conn->c_conn_state = SLAP_C_CLOSING;
+                       connection_closing( conn );

 
                        ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
                        ldap_pvt_thread_mutex_unlock( &conn->c_write_mutex );
 
                        ldap_pvt_thread_mutex_unlock( &conn->c_mutex );
                        ldap_pvt_thread_mutex_unlock( &conn->c_write_mutex );

diff --git a/servers/slapd/slap.h b/servers/slapd/slap.h
index dabf8c7bec159ad75ea5fe168e462bdab9e2c979..72503912e18086ccc685a96844852bd5c85a90ec 100644 (file)
--- a/servers/slapd/slap.h
+++ b/servers/slapd/slap.h
@@ -414,19 +414,6 @@ typedef struct slap_op {
  * represents a connection from an ldap client
  */
 
  * represents a connection from an ldap client
  */
 

- 
-/* structure state (protected by connections_mutex) */
-#define SLAP_C_UNINITIALIZED   0x0     /* MUST BE ZERO (0) */
-#define SLAP_C_UNUSED                  0x1
-#define SLAP_C_USED                            0x2
-
-/* connection state (protected by c_mutex ) */
-#define SLAP_C_INVALID                 0x0     /* MUST BE ZERO (0) */
-#define SLAP_C_INACTIVE                        0x1     /* zero threads */
-#define SLAP_C_ACTIVE                  0x2 /* one or more threads */
-#define SLAP_C_BINDING                 0x3     /* binding */
-#define SLAP_C_CLOSING                 0x4     /* closing */
-

 typedef struct slap_conn {
        int                     c_struct_state; /* structure management state */
        int                     c_conn_state;   /* connection state */
 typedef struct slap_conn {
        int                     c_struct_state; /* structure management state */
        int                     c_conn_state;   /* connection state */