ITS#9001 Use a TAvl for request tracking in libldap

diff --git a/libraries/libldap/abandon.c b/libraries/libldap/abandon.c
index d88f63a849398a59e29cf3d2d2baba8f8b7f105c..9c5ee043a9b7321031b009e3b2abce936218fdf2 100644 (file)
--- a/libraries/libldap/abandon.c
+++ b/libraries/libldap/abandon.c
@@ -40,7 +40,7 @@ static int
 do_abandon(
        LDAP *ld,
        ber_int_t origid,
-       ber_int_t msgid,
+       LDAPRequest *lr,
        LDAPControl **sctrls,
        int sendabandon );
 
@@ -75,7 +75,7 @@ ldap_abandon_ext(
 
        rc = ldap_int_client_controls( ld, cctrls );
        if ( rc == LDAP_SUCCESS ) {
-               rc = do_abandon( ld, msgid, msgid, sctrls, 1 );
+               rc = do_abandon( ld, msgid, NULL, sctrls, 1 );
        }
 
        LDAP_MUTEX_UNLOCK( &ld->ld_req_mutex );
@@ -112,7 +112,7 @@ ldap_pvt_discard(
        int     rc;
 
        LDAP_MUTEX_LOCK( &ld->ld_req_mutex );
-       rc = do_abandon( ld, msgid, msgid, NULL, 0 );
+       rc = do_abandon( ld, msgid, NULL, NULL, 0 );
        LDAP_MUTEX_UNLOCK( &ld->ld_req_mutex );
        return rc;
 }
@@ -121,49 +121,52 @@ static int
 do_abandon(
        LDAP *ld,
        ber_int_t origid,
-       ber_int_t msgid,
+       LDAPRequest *lr,
        LDAPControl **sctrls,
        int sendabandon )
 {
        BerElement      *ber;
        int             i, err;
+       ber_int_t       msgid = origid;
        Sockbuf         *sb;
-       LDAPRequest     *lr;
-
-       Debug2( LDAP_DEBUG_TRACE, "do_abandon origid %d, msgid %d\n",
-               origid, msgid );
-
-       /* find the request that we are abandoning */
-start_again:;
-       lr = ld->ld_requests;
-       while ( lr != NULL ) {
-               /* this message */
-               if ( lr->lr_msgid == msgid ) {
-                       break;
-               }
-
-               /* child: abandon it */
-               if ( lr->lr_origid == msgid && !lr->lr_abandoned ) {
-                       (void)do_abandon( ld, lr->lr_origid, lr->lr_msgid,
-                               sctrls, sendabandon );
-
-                       /* restart, as lr may now be dangling... */
-                       goto start_again;
-               }
+       LDAPRequest     needle = {0};
 
-               lr = lr->lr_next;
-       }
+       needle.lr_msgid = origid;
 
        if ( lr != NULL ) {
-               if ( origid == msgid && lr->lr_parent != NULL ) {
+               msgid = lr->lr_msgid;
+               Debug2( LDAP_DEBUG_TRACE, "do_abandon origid %d, msgid %d\n",
+                               origid, msgid );
+       } else if ( (lr = ldap_tavl_find( ld->ld_requests, &needle, ldap_req_cmp )) != NULL ) {
+               Debug2( LDAP_DEBUG_TRACE, "do_abandon origid %d, msgid %d\n",
+                               origid, msgid );
+               if ( lr->lr_parent != NULL ) {
                        /* don't let caller abandon child requests! */
                        ld->ld_errno = LDAP_PARAM_ERROR;
                        return( LDAP_PARAM_ERROR );
                }
+               msgid = lr->lr_msgid;
+       }
+
+       if ( lr != NULL ) {
+               LDAPRequest **childp = &lr->lr_child;
+
+               needle.lr_msgid = lr->lr_msgid;
+
                if ( lr->lr_status != LDAP_REQST_INPROGRESS ) {
                        /* no need to send abandon message */
                        sendabandon = 0;
                }
+
+               while ( *childp ) {
+                       /* Abandon children */
+                       LDAPRequest *child = *childp;
+
+                       (void)do_abandon( ld, lr->lr_origid, child, sctrls, sendabandon );
+                       if ( *childp == child ) {
+                               childp = &child->lr_refnext;
+                       }
+               }
        }
 
        /* ldap_msgdelete locks the res_mutex. Give up the req_mutex
@@ -179,12 +182,7 @@ start_again:;
 
        /* fetch again the request that we are abandoning */
        if ( lr != NULL ) {
-               for ( lr = ld->ld_requests; lr != NULL; lr = lr->lr_next ) {
-                       /* this message */
-                       if ( lr->lr_msgid == msgid ) {
-                               break;
-                       }
-               }
+               lr = ldap_tavl_find( ld->ld_requests, &needle, ldap_req_cmp );
        }
 
        err = 0;

diff --git a/libraries/libldap/ldap-int.h b/libraries/libldap/ldap-int.h
index e40518b32d48764fff510570c3790bfa6d076cd1..06268a96f29bb31a60bc0546cd2befb39b25d980 100644 (file)
--- a/libraries/libldap/ldap-int.h
+++ b/libraries/libldap/ldap-int.h
@@ -26,6 +26,7 @@
 
 #include "../liblber/lber-int.h"
 #include "lutil.h"
+#include "ldap_avl.h"
 
 #ifdef LDAP_R_COMPILE
 #include <ldap_pvt_thread.h>
@@ -438,7 +439,7 @@ struct ldap_common {
 
        /* do not mess with these */
        /* protected by req_mutex */
-       LDAPRequest     *ldc_requests;  /* list of outstanding requests */
+       TAvlnode        *ldc_requests;  /* list of outstanding requests */
        /* protected by res_mutex */
        LDAPMessage     *ldc_responses; /* list of outstanding responses */
 #define        ld_requests             ldc->ldc_requests
@@ -776,6 +777,8 @@ LDAP_F (LDAPConn *) ldap_new_connection( LDAP *ld, LDAPURLDesc **srvlist,
        int use_ldsb, int connect, LDAPreqinfo *bind, int m_req, int m_res );
 LDAP_F (LDAPRequest *) ldap_find_request_by_msgid( LDAP *ld, ber_int_t msgid );
 LDAP_F (void) ldap_return_request( LDAP *ld, LDAPRequest *lr, int freeit );
+LDAP_F (int) ldap_req_cmp( const void *l, const void *r );
+LDAP_F (void) ldap_do_free_request( void *arg );
 LDAP_F (void) ldap_free_request( LDAP *ld, LDAPRequest *lr );
 LDAP_F (void) ldap_free_connection( LDAP *ld, LDAPConn *lc, int force, int unbind );
 LDAP_F (void) ldap_dump_connection( LDAP *ld, LDAPConn *lconns, int all );

diff --git a/libraries/libldap/open.c b/libraries/libldap/open.c
index fdded08149629cf71f94db24090300f4cbb9bfc7..1289ced90775c65f4664d6ff2fc5e7fa4f8cc3b0 100644 (file)
--- a/libraries/libldap/open.c
+++ b/libraries/libldap/open.c
@@ -574,7 +574,8 @@ ldap_open_internal_connection( LDAP **ldp, ber_socket_t *fdp )
        lr->lr_status = LDAP_REQST_INPROGRESS;
        lr->lr_res_errno = LDAP_SUCCESS;
        /* no mutex lock needed, we just created this ld here */
-       ld->ld_requests = lr;
+       rc = ldap_tavl_insert( &ld->ld_requests, lr, ldap_req_cmp, ldap_avl_dup_error );
+       assert( rc == LDAP_SUCCESS );
 
        LDAP_MUTEX_LOCK( &ld->ld_conn_mutex );
        /* Attach the passed socket as the *LDAP's connection */

diff --git a/libraries/libldap/request.c b/libraries/libldap/request.c
index e60139e8e06e5555bdb5349634e8e8411dea825d..73ce725b8ab24d6a24095137786df03ffa0d3d19 100644 (file)
--- a/libraries/libldap/request.c
+++ b/libraries/libldap/request.c
@@ -328,11 +328,16 @@ ldap_send_server_request(
         * request to be in WRITING state.
         */
        rc = 0;
-       if ( ld->ld_requests &&
-               ld->ld_requests->lr_status == LDAP_REQST_WRITING &&
-               ldap_int_flush_request( ld, ld->ld_requests ) < 0 )
-       {
-               rc = -1;
+       if ( ld->ld_requests != NULL ) {
+               TAvlnode *node = ldap_tavl_end( ld->ld_requests, TAVL_DIR_RIGHT );
+               LDAPRequest *lr;
+
+               assert( node != NULL );
+               lr = node->avl_data;
+               if ( lr->lr_status == LDAP_REQST_WRITING &&
+                               ldap_int_flush_request( ld, lr ) < 0 ) {
+                       rc = -1;
+               }
        }
        if ( rc ) {
                ber_free( ber, 1 );
@@ -399,12 +404,8 @@ ldap_send_server_request(
                }
        }
 
-       lr->lr_prev = NULL;
-       lr->lr_next = ld->ld_requests;
-       if ( lr->lr_next != NULL ) {
-               lr->lr_next->lr_prev = lr;
-       }
-       ld->ld_requests = lr;
+       rc = ldap_tavl_insert( &ld->ld_requests, lr, ldap_req_cmp, ldap_avl_dup_error );
+       assert( rc == LDAP_SUCCESS );
 
        ld->ld_errno = LDAP_SUCCESS;
        if ( ldap_int_flush_request( ld, lr ) == -1 ) {
@@ -803,19 +804,10 @@ ldap_free_connection( LDAP *ld, LDAPConn *lc, int force, int unbind )
                /* FIXME: is this at all possible?
                 * ldap_ld_free() in unbind.c calls ldap_free_connection()
                 * with force == 1 __after__ explicitly calling
-                * ldap_free_request() on all requests */
+                * ldap_tavl_free on ld->ld_requests */
                if ( force ) {
-                       LDAPRequest     *lr;
-
-                       for ( lr = ld->ld_requests; lr; ) {
-                               LDAPRequest     *lr_next = lr->lr_next;
-
-                               if ( lr->lr_conn == lc ) {
-                                       ldap_free_request_int( ld, lr );
-                               }
-
-                               lr = lr_next;
-                       }
+                       ldap_tavl_free( ld->ld_requests, ldap_do_free_request );
+                       ld->ld_requests = NULL;
                }
 
                if ( lc->lconn_sb != ld->ld_sb ) {
@@ -913,17 +905,19 @@ ldap_dump_connection( LDAP *ld, LDAPConn *lconns, int all )
 void
 ldap_dump_requests_and_responses( LDAP *ld )
 {
-       LDAPRequest     *lr;
        LDAPMessage     *lm, *l;
+       TAvlnode *node;
        int             i;
 
        Debug1( LDAP_DEBUG_TRACE, "** ld %p Outstanding Requests:\n",
                (void *)ld );
-       lr = ld->ld_requests;
-       if ( lr == NULL ) {
+       node = ldap_tavl_end( ld->ld_requests, TAVL_DIR_LEFT );
+       if ( node == NULL ) {
                Debug0( LDAP_DEBUG_TRACE, "   Empty\n" );
        }
-       for ( i = 0; lr != NULL; lr = lr->lr_next, i++ ) {
+       for ( i = 0 ; node != NULL; i++, node = ldap_tavl_next( node, TAVL_DIR_RIGHT ) ) {
+               LDAPRequest     *lr = node->avl_data;
+
                Debug3( LDAP_DEBUG_TRACE, " * msgid %d,  origid %d, status %s\n",
                        lr->lr_msgid, lr->lr_origid,
                        ( lr->lr_status == LDAP_REQST_INPROGRESS ) ? "InProgress" :
@@ -959,39 +953,21 @@ ldap_dump_requests_and_responses( LDAP *ld )
 #endif /* LDAP_DEBUG */
 
 /* protected by req_mutex */
-static void
-ldap_free_request_int( LDAP *ld, LDAPRequest *lr )
+void
+ldap_do_free_request( void *arg )
 {
-       LDAP_ASSERT_MUTEX_OWNER( &ld->ld_req_mutex );
+       LDAPRequest *lr = arg;
+
+       Debug3( LDAP_DEBUG_TRACE, "ldap_do_free_request: "
+                       "asked to free lr %p msgid %d refcnt %d\n",
+                       lr, lr->lr_msgid, lr->lr_refcnt );
        /* if lr_refcnt > 0, the request has been looked up 
         * by ldap_find_request_by_msgid(); if in the meanwhile
         * the request is free()'d by someone else, just decrease
-        * the reference count and extract it from the request
-        * list; later on, it will be freed. */
-       if ( lr->lr_prev == NULL ) {
-               if ( lr->lr_refcnt == 0 ) {
-                       /* free'ing the first request? */
-                       assert( ld->ld_requests == lr );
-               }
-
-               if ( ld->ld_requests == lr ) {
-                       ld->ld_requests = lr->lr_next;
-               }
-
-       } else {
-               lr->lr_prev->lr_next = lr->lr_next;
-       }
-
-       if ( lr->lr_next != NULL ) {
-               lr->lr_next->lr_prev = lr->lr_prev;
-       }
-
+        * the reference count; later on, it will be freed. */
        if ( lr->lr_refcnt > 0 ) {
+               assert( lr->lr_refcnt == 1 );
                lr->lr_refcnt = -lr->lr_refcnt;
-
-               lr->lr_prev = NULL;
-               lr->lr_next = NULL;
-
                return;
        }
 
@@ -1013,6 +989,29 @@ ldap_free_request_int( LDAP *ld, LDAPRequest *lr )
        LDAP_FREE( lr );
 }
 
+int
+ldap_req_cmp( const void *l, const void *r )
+{
+       const LDAPRequest *left = l, *right = r;
+       return left->lr_msgid - right->lr_msgid;
+}
+
+/* protected by req_mutex */
+static void
+ldap_free_request_int( LDAP *ld, LDAPRequest *lr )
+{
+       LDAPRequest *removed;
+
+       LDAP_ASSERT_MUTEX_OWNER( &ld->ld_req_mutex );
+       removed = ldap_tavl_delete( &ld->ld_requests, lr, ldap_req_cmp );
+       assert( !removed || removed == lr );
+       Debug3( LDAP_DEBUG_TRACE, "ldap_free_request_int: "
+                       "lr %p msgid %d%s removed\n",
+                       lr, lr->lr_msgid, removed ? "" : " not" );
+
+       ldap_do_free_request( lr );
+}
+
 /* protected by req_mutex */
 void
 ldap_free_request( LDAP *ld, LDAPRequest *lr )
@@ -1662,19 +1661,24 @@ re_encode_request( LDAP *ld,
 LDAPRequest *
 ldap_find_request_by_msgid( LDAP *ld, ber_int_t msgid )
 {
-       LDAPRequest     *lr;
-
-       for ( lr = ld->ld_requests; lr != NULL; lr = lr->lr_next ) {
-               if ( lr->lr_status == LDAP_REQST_COMPLETED ) {
-                       continue;       /* Skip completed requests */
-               }
-               if ( msgid == lr->lr_msgid ) {
-                       lr->lr_refcnt++;
-                       break;
-               }
+       LDAPRequest     *lr, needle = {0};
+       needle.lr_msgid = msgid;
+
+       lr = ldap_tavl_find( ld->ld_requests, &needle, ldap_req_cmp );
+       if ( lr != NULL && lr->lr_status != LDAP_REQST_COMPLETED ) {
+               /* try_read1msg is the only user at the moment and we would free it
+                * multiple times if retrieving the request again */
+               assert( lr->lr_refcnt == 0 );
+               lr->lr_refcnt++;
+               Debug3( LDAP_DEBUG_TRACE, "ldap_find_request_by_msgid: "
+                               "msgid %d, lr %p lr->lr_refcnt = %d\n",
+                               msgid, lr, lr->lr_refcnt );
+               return lr;
        }
 
-       return( lr );
+       Debug2( LDAP_DEBUG_TRACE, "ldap_find_request_by_msgid: "
+                       "msgid %d, lr %p\n", msgid, lr );
+       return NULL;
 }
 
 /* protected by req_mutex */
@@ -1683,23 +1687,26 @@ ldap_return_request( LDAP *ld, LDAPRequest *lrx, int freeit )
 {
        LDAPRequest     *lr;
 
-       for ( lr = ld->ld_requests; lr != NULL; lr = lr->lr_next ) {
-               if ( lr == lrx ) {
-                       if ( lr->lr_refcnt > 0 ) {
-                               lr->lr_refcnt--;
-
-                       } else if ( lr->lr_refcnt < 0 ) {
-                               lr->lr_refcnt++;
-                               if ( lr->lr_refcnt == 0 ) {
-                                       lr = NULL;
-                               }
+       lr = ldap_tavl_find( ld->ld_requests, lrx, ldap_req_cmp );
+       Debug2( LDAP_DEBUG_TRACE, "ldap_return_request: "
+                       "lrx %p, lr %p\n", lrx, lr );
+       if ( lr ) {
+               assert( lr == lrx );
+               if ( lr->lr_refcnt > 0 ) {
+                       lr->lr_refcnt--;
+               } else if ( lr->lr_refcnt < 0 ) {
+                       lr->lr_refcnt++;
+                       if ( lr->lr_refcnt == 0 ) {
+                               lr = NULL;
                        }
-                       break;
                }
        }
+       Debug3( LDAP_DEBUG_TRACE, "ldap_return_request: "
+                       "lrx->lr_msgid %d, lrx->lr_refcnt is now %d, lr is %s present\n",
+                       lrx->lr_msgid, lrx->lr_refcnt, lr ? "still" : "not" );
+       /* The request is not tracked anymore */
        if ( lr == NULL ) {
                ldap_free_request_int( ld, lrx );
-
        } else if ( freeit ) {
                ldap_free_request( ld, lrx );
        }

diff --git a/libraries/libldap/result.c b/libraries/libldap/result.c
index a6977caefb728d7235d296797d86865149a5173c..bc47ddafe00e40e3827c445fde6f9cd2d3130dc6 100644 (file)
--- a/libraries/libldap/result.c
+++ b/libraries/libldap/result.c
@@ -344,13 +344,17 @@ wait4msg(
                                int serviced = 0;
                                rc = LDAP_MSG_X_KEEP_LOOKING;
                                LDAP_MUTEX_LOCK( &ld->ld_req_mutex );
-                               if ( ld->ld_requests &&
-                                       ld->ld_requests->lr_status == LDAP_REQST_WRITING &&
-                                       ldap_is_write_ready( ld,
-                                               ld->ld_requests->lr_conn->lconn_sb ) )
-                               {
-                                       serviced = 1;
-                                       ldap_int_flush_request( ld, ld->ld_requests );
+                               if ( ld->ld_requests != NULL ) {
+                                       TAvlnode *node = ldap_tavl_end( ld->ld_requests, TAVL_DIR_RIGHT );
+                                       LDAPRequest *lr;
+
+                                       assert( node != NULL );
+                                       lr = node->avl_data;
+                                       if ( lr->lr_status == LDAP_REQST_WRITING &&
+                                                       ldap_is_write_ready( ld, lr->lr_conn->lconn_sb ) ) {
+                                               serviced = 1;
+                                               ldap_int_flush_request( ld, lr );
+                                       }
                                }
                                for ( lc = ld->ld_conns;
                                        rc == LDAP_MSG_X_KEEP_LOOKING && lc != NULL;
@@ -452,7 +456,7 @@ try_read1msg(
        LDAPRequest     *lr, *tmplr, dummy_lr = { 0 };
        BerElement      tmpber;
        int             rc, refer_cnt, hadref, simple_request, err;
-       ber_int_t       lderr;
+       ber_int_t       lderr = -1;
 
 #ifdef LDAP_CONNECTIONLESS
        LDAPMessage     *tmp = NULL, *chain_head = NULL;

diff --git a/libraries/libldap/unbind.c b/libraries/libldap/unbind.c
index 51f85b81621b5cfe970b2dc9f715a65bff06b06e..ca89930d1266d7eaa9b43a6d286f1e458cd59b0f 100644 (file)
--- a/libraries/libldap/unbind.c
+++ b/libraries/libldap/unbind.c
@@ -108,9 +108,8 @@ ldap_ld_free(
 
        /* free LDAP structure and outstanding requests/responses */
        LDAP_MUTEX_LOCK( &ld->ld_req_mutex );
-       while ( ld->ld_requests != NULL ) {
-               ldap_free_request( ld, ld->ld_requests );
-       }
+       ldap_tavl_free( ld->ld_requests, ldap_do_free_request );
+       ld->ld_requests = NULL;
        LDAP_MUTEX_UNLOCK( &ld->ld_req_mutex );
        LDAP_MUTEX_LOCK( &ld->ld_conn_mutex );