First cut at bind race fix. Passes our test suite at least.

diff --git a/servers/slapd/connection.c b/servers/slapd/connection.c
index 7246a17b2f6199a471734d1c4d1b35420c7e01b1..75871477337b160daaa42905b991316ab66d4743 100644 (file)
--- a/servers/slapd/connection.c
+++ b/servers/slapd/connection.c
@@ -10,6 +10,9 @@
 
 #include "slap.h"
 
+static int connection_op_activate( Connection *conn, Operation *op );
+static int connection_resched( Connection *conn );
+
 struct co_arg {
        Connection      *co_conn;
        Operation       *co_op;
@@ -25,59 +28,61 @@ static void *
 connection_operation( void *arg_v )
 {
        struct co_arg   *arg = arg_v;
+       int tag = arg->co_op->o_tag;
+       Connection *conn = arg->co_conn;
 
-       ldap_pvt_thread_mutex_lock( &arg->co_conn->c_opsmutex );
-       arg->co_conn->c_ops_received++;
-       ldap_pvt_thread_mutex_unlock( &arg->co_conn->c_opsmutex );
+       ldap_pvt_thread_mutex_lock( &conn->c_opsmutex );
+       conn->c_ops_received++;
+       ldap_pvt_thread_mutex_unlock( &conn->c_opsmutex );
 
        ldap_pvt_thread_mutex_lock( &ops_mutex );
        ops_initiated++;
        ldap_pvt_thread_mutex_unlock( &ops_mutex );
 
-       switch ( arg->co_op->o_tag ) {
+       switch ( tag ) {
        case LDAP_REQ_BIND:
-               do_bind( arg->co_conn, arg->co_op );
+               do_bind( conn, arg->co_op );
                break;
 
 #ifdef LDAP_COMPAT30
        case LDAP_REQ_UNBIND_30:
 #endif
        case LDAP_REQ_UNBIND:
-               do_unbind( arg->co_conn, arg->co_op );
+               do_unbind( conn, arg->co_op );
                break;
 
        case LDAP_REQ_ADD:
-               do_add( arg->co_conn, arg->co_op );
+               do_add( conn, arg->co_op );
                break;
 
 #ifdef LDAP_COMPAT30
        case LDAP_REQ_DELETE_30:
 #endif
        case LDAP_REQ_DELETE:
-               do_delete( arg->co_conn, arg->co_op );
+               do_delete( conn, arg->co_op );
                break;
 
        case LDAP_REQ_MODRDN:
-               do_modrdn( arg->co_conn, arg->co_op );
+               do_modrdn( conn, arg->co_op );
                break;
 
        case LDAP_REQ_MODIFY:
-               do_modify( arg->co_conn, arg->co_op );
+               do_modify( conn, arg->co_op );
                break;
 
        case LDAP_REQ_COMPARE:
-               do_compare( arg->co_conn, arg->co_op );
+               do_compare( conn, arg->co_op );
                break;
 
        case LDAP_REQ_SEARCH:
-               do_search( arg->co_conn, arg->co_op );
+               do_search( conn, arg->co_op );
                break;
 
 #ifdef LDAP_COMPAT30
        case LDAP_REQ_ABANDON_30:
 #endif
        case LDAP_REQ_ABANDON:
-               do_abandon( arg->co_conn, arg->co_op );
+               do_abandon( conn, arg->co_op );
                break;
 
        default:
@@ -86,20 +91,25 @@ connection_operation( void *arg_v )
                break;
        }
 
-       ldap_pvt_thread_mutex_lock( &arg->co_conn->c_opsmutex );
-       arg->co_conn->c_ops_completed++;
-
-       slap_op_delete( &arg->co_conn->c_ops, arg->co_op );
-       arg->co_op = NULL;
+       ldap_pvt_thread_mutex_lock( &ops_mutex );
+       ops_completed++;
+       ldap_pvt_thread_mutex_unlock( &ops_mutex );
 
-       ldap_pvt_thread_mutex_unlock( &arg->co_conn->c_opsmutex );
+       ldap_pvt_thread_mutex_lock( &conn->c_opsmutex );
+       conn->c_ops_completed++;
 
+       slap_op_remove( &conn->c_ops, arg->co_op );
+       slap_op_free( arg->co_op );
+       arg->co_op = NULL;
        arg->co_conn = NULL;
        free( (char *) arg );
+       arg = NULL;
 
-       ldap_pvt_thread_mutex_lock( &ops_mutex );
-       ops_completed++;
-       ldap_pvt_thread_mutex_unlock( &ops_mutex );
+       if((tag == LDAP_REQ_BIND) && (conn->c_state == SLAP_C_BINDING)) {
+               conn->c_state = SLAP_C_ACTIVE;
+       }
+
+       ldap_pvt_thread_mutex_unlock( &conn->c_opsmutex );
 
        ldap_pvt_thread_mutex_lock( &active_threads_mutex );
        active_threads--;
@@ -107,6 +117,9 @@ connection_operation( void *arg_v )
                ldap_pvt_thread_cond_signal(&active_threads_cond);
        }
        ldap_pvt_thread_mutex_unlock( &active_threads_mutex );
+
+       connection_resched( conn );
+
        return NULL;
 }
 
@@ -115,12 +128,10 @@ connection_activity(
     Connection *conn
 )
 {
-       int status;
-       struct co_arg   *arg;
+       Operation *op;
        unsigned long   tag, len;
        long            msgid;
        BerElement      *ber;
-       char            *tmpdn;
 
        if ( conn->c_currentber == NULL && (conn->c_currentber = ber_alloc())
            == NULL ) {
@@ -178,8 +189,60 @@ connection_activity(
        }
 #endif
 
-       arg = (struct co_arg *) ch_malloc( sizeof(struct co_arg) );
-       arg->co_conn = conn;
+       op = slap_op_alloc( ber, msgid, tag,
+               conn->c_ops_received, conn->c_connid );
+
+       if ( conn->c_state == SLAP_C_BINDING ) {
+               /* connection is binding to a dn, make 'em wait */
+               ldap_pvt_thread_mutex_lock( &conn->c_opsmutex );
+               slap_op_add( &conn->c_pending_ops, op );
+
+               Debug( LDAP_DEBUG_ANY, "deferring operation\n", 0, 0, 0 );
+
+               ldap_pvt_thread_mutex_unlock( &conn->c_opsmutex );
+
+               return;
+       }
+
+       connection_op_activate( conn, op );
+}
+
+static int
+connection_resched( Connection *conn )
+{
+       Operation *op;
+
+       if( conn->c_state != SLAP_C_ACTIVE ) {
+               /* other states need different handling */
+               return;
+       }
+
+       ldap_pvt_thread_mutex_lock( &conn->c_opsmutex );
+
+       for( op = slap_op_pop( &conn->c_pending_ops );
+               op != NULL;
+               op = slap_op_pop( &conn->c_pending_ops ) )
+       {
+               ldap_pvt_thread_mutex_unlock( &conn->c_opsmutex );
+
+               connection_op_activate( conn, op );
+
+               ldap_pvt_thread_mutex_lock( &conn->c_opsmutex );
+
+               if ( conn->c_state == SLAP_C_BINDING ) {
+                       break;
+               }
+       }
+
+       ldap_pvt_thread_mutex_unlock( &conn->c_opsmutex );
+}
+
+static int connection_op_activate( Connection *conn, Operation *op )
+{
+       struct co_arg *arg;
+       char *tmpdn;
+       int status;
+       unsigned long tag = op->o_tag;
 
        ldap_pvt_thread_mutex_lock( &conn->c_dnmutex );
        if ( conn->c_dn != NULL ) {
@@ -189,9 +252,21 @@ connection_activity(
        }
        ldap_pvt_thread_mutex_unlock( &conn->c_dnmutex );
 
+       arg = (struct co_arg *) ch_malloc( sizeof(struct co_arg) );
+       arg->co_conn = conn;
+       arg->co_op = op;
+
+       arg->co_op->o_dn = ch_strdup( tmpdn != NULL ? tmpdn : "" );
+       arg->co_op->o_ndn = dn_normalize_case( ch_strdup( arg->co_op->o_dn ) );
+
        ldap_pvt_thread_mutex_lock( &conn->c_opsmutex );
-       arg->co_op = slap_op_add( &conn->c_ops, ber, msgid, tag, tmpdn,
-           conn->c_ops_received, conn->c_connid );
+
+       slap_op_add( &conn->c_ops, arg->co_op );
+
+       if(tag == LDAP_REQ_BIND) {
+               conn->c_state = SLAP_C_BINDING;
+       }
+
        ldap_pvt_thread_mutex_unlock( &conn->c_opsmutex );
 
        if ( tmpdn != NULL ) {
@@ -204,7 +279,10 @@ connection_activity(
 
        status = ldap_pvt_thread_create( &arg->co_op->o_tid, 1,
                                         connection_operation, (void *) arg );
+
        if ( status != 0 ) {
                Debug( LDAP_DEBUG_ANY, "ldap_pvt_thread_create failed (%d)\n", status, 0, 0 );
        }
+
+       return status;
 }

diff --git a/servers/slapd/operation.c b/servers/slapd/operation.c
index 2fff851887656720e1705ec74cb54f7982fbf770..404e20d888c1b48a5b4df5c1dac178e332ceb52d 100644 (file)
--- a/servers/slapd/operation.c
+++ b/servers/slapd/operation.c
@@ -13,6 +13,10 @@
 void
 slap_op_free( Operation *op )
 {
+#ifdef LDAP_DEBUG
+       assert( op->o_next == NULL );
+#endif
+
        ldap_pvt_thread_mutex_lock( &op->o_abandonmutex );
 
        if ( op->o_ber != NULL ) {
@@ -31,44 +35,54 @@ slap_op_free( Operation *op )
 }
 
 Operation *
-slap_op_add(
-    Operation          **olist,
+slap_op_alloc(
     BerElement         *ber,
     unsigned long      msgid,
     unsigned long      tag,
-    char                       *dn,
     int                                id,
     int                                connid
 )
 {
-       Operation       **tmp;
+       Operation       *op;
 
-       for ( tmp = olist; *tmp != NULL; tmp = &(*tmp)->o_next )
-               ;       /* NULL */
-
-       *tmp = (Operation *) ch_calloc( 1, sizeof(Operation) );
+       op = (Operation *) ch_calloc( 1, sizeof(Operation) );
 
-       ldap_pvt_thread_mutex_init( &(*tmp)->o_abandonmutex );
-       (*tmp)->o_ber = ber;
-       (*tmp)->o_msgid = msgid;
-       (*tmp)->o_tag = tag;
-       (*tmp)->o_abandon = 0;
+       ldap_pvt_thread_mutex_init( &op->o_abandonmutex );
+       op->o_ber = ber;
+       op->o_msgid = msgid;
+       op->o_tag = tag;
+       op->o_abandon = 0;
 
-       (*tmp)->o_dn = ch_strdup( dn != NULL ? dn : "" );
-       (*tmp)->o_ndn = dn_normalize_case( ch_strdup( (*tmp)->o_dn ) );
+       op->o_dn = NULL;
+       op->o_ndn = NULL;
 
        ldap_pvt_thread_mutex_lock( &currenttime_mutex );
-       (*tmp)->o_time = currenttime;
+       op->o_time = currenttime;
        ldap_pvt_thread_mutex_unlock( &currenttime_mutex );
-       (*tmp)->o_opid = id;
-       (*tmp)->o_connid = connid;
-       (*tmp)->o_next = NULL;
+       op->o_opid = id;
+       op->o_connid = connid;
+       op->o_next = NULL;
 
-       return( *tmp );
+       return( op );
 }
 
-void
-slap_op_delete( Operation **olist, Operation *op )
+int slap_op_add(
+    Operation          **olist,
+       Operation               *op
+)
+{
+       Operation       **tmp;
+
+       for ( tmp = olist; *tmp != NULL; tmp = &(*tmp)->o_next )
+               ;       /* NULL */
+
+       *tmp = op;
+
+       return 0;
+}
+
+int
+slap_op_remove( Operation **olist, Operation *op )
 {
        Operation       **tmp;
 
@@ -78,10 +92,24 @@ slap_op_delete( Operation **olist, Operation *op )
        if ( *tmp == NULL ) {
                Debug( LDAP_DEBUG_ANY, "op_delete: can't find op %ld\n",
                    op->o_msgid, 0, 0 );
-               slap_op_free( op );
-               return; 
+               return -1; 
        }
 
        *tmp = (*tmp)->o_next;
-       slap_op_free( op );
+       op->o_next = NULL;
+
+       return 0;
 }
+
+Operation * slap_op_pop( Operation **olist )
+{
+       Operation *tmp = *olist;
+
+       if(tmp != NULL) {
+               *olist = tmp->o_next;
+               tmp->o_next = NULL;
+       }
+
+       return tmp;
+}
+

diff --git a/servers/slapd/proto-slap.h b/servers/slapd/proto-slap.h
index b3b1a9737a48d202b3b94b22c3865b7de965b900..18fd25614be9f8ed5ab61a4a684daf76b137cbf2 100644 (file)
--- a/servers/slapd/proto-slap.h
+++ b/servers/slapd/proto-slap.h
@@ -171,10 +171,13 @@ void monitor_info LDAP_P(( Connection *conn, Operation *op ));
  */
 
 void slap_op_free LDAP_P(( Operation *op ));
-Operation * slap_op_add LDAP_P(( Operation **olist,
+Operation * slap_op_alloc LDAP_P((
        BerElement *ber, unsigned long msgid,
-       unsigned long tag, char *dn, int id, int connid ));
-void slap_op_delete LDAP_P(( Operation **olist, Operation *op ));
+       unsigned long tag, int id, int connid ));
+
+int slap_op_add LDAP_P(( Operation **olist, Operation *op ));
+int slap_op_remove LDAP_P(( Operation **olist, Operation *op ));
+Operation * slap_op_pop LDAP_P(( Operation **olist ));
 
 /*
  * phonetic.c

diff --git a/servers/slapd/slap.h b/servers/slapd/slap.h
index 3457eddc804bb44198b10454f039de3c339bb55d..c9e9ad7338446f2549821325f994b850f31b9267 100644 (file)
--- a/servers/slapd/slap.h
+++ b/servers/slapd/slap.h
@@ -399,7 +399,7 @@ typedef struct slap_op {
        struct sockaddr o_clientaddr;   /* client address if via CLDAP    */
        char            o_searchbase;   /* search base if via CLDAP       */
 #endif
-       struct slap_op  *o_next;        /* next operation pending         */
+       struct slap_op  *o_next;        /* next operation in list         */
        ldap_pvt_thread_t       o_tid;          /* thread handling this op        */
        int             o_abandon;      /* signals op has been abandoned  */
        ldap_pvt_thread_mutex_t o_abandonmutex; /* signals op has been abandoned  */