Commit 3bd1b090 authored by Ondřej Kuzník's avatar Ondřej Kuzník Committed by Ondřej Kuzník
Browse files

ITS#9001 Use a TAvl for request tracking in libldap

parent e36d1e31
...@@ -40,7 +40,7 @@ static int ...@@ -40,7 +40,7 @@ static int
do_abandon( do_abandon(
LDAP *ld, LDAP *ld,
ber_int_t origid, ber_int_t origid,
ber_int_t msgid, LDAPRequest *lr,
LDAPControl **sctrls, LDAPControl **sctrls,
int sendabandon ); int sendabandon );
...@@ -75,7 +75,7 @@ ldap_abandon_ext( ...@@ -75,7 +75,7 @@ ldap_abandon_ext(
rc = ldap_int_client_controls( ld, cctrls ); rc = ldap_int_client_controls( ld, cctrls );
if ( rc == LDAP_SUCCESS ) { 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 ); LDAP_MUTEX_UNLOCK( &ld->ld_req_mutex );
...@@ -112,7 +112,7 @@ ldap_pvt_discard( ...@@ -112,7 +112,7 @@ ldap_pvt_discard(
int rc; int rc;
LDAP_MUTEX_LOCK( &ld->ld_req_mutex ); 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 ); LDAP_MUTEX_UNLOCK( &ld->ld_req_mutex );
return rc; return rc;
} }
...@@ -121,49 +121,52 @@ static int ...@@ -121,49 +121,52 @@ static int
do_abandon( do_abandon(
LDAP *ld, LDAP *ld,
ber_int_t origid, ber_int_t origid,
ber_int_t msgid, LDAPRequest *lr,
LDAPControl **sctrls, LDAPControl **sctrls,
int sendabandon ) int sendabandon )
{ {
BerElement *ber; BerElement *ber;
int i, err; int i, err;
ber_int_t msgid = origid;
Sockbuf *sb; Sockbuf *sb;
LDAPRequest *lr; LDAPRequest needle = {0};
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;
}
lr = lr->lr_next; needle.lr_msgid = origid;
}
if ( lr != NULL ) { 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! */ /* don't let caller abandon child requests! */
ld->ld_errno = LDAP_PARAM_ERROR; ld->ld_errno = LDAP_PARAM_ERROR;
return( 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 ) { if ( lr->lr_status != LDAP_REQST_INPROGRESS ) {
/* no need to send abandon message */ /* no need to send abandon message */
sendabandon = 0; 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 /* ldap_msgdelete locks the res_mutex. Give up the req_mutex
...@@ -179,12 +182,7 @@ start_again:; ...@@ -179,12 +182,7 @@ start_again:;
/* fetch again the request that we are abandoning */ /* fetch again the request that we are abandoning */
if ( lr != NULL ) { if ( lr != NULL ) {
for ( lr = ld->ld_requests; lr != NULL; lr = lr->lr_next ) { lr = ldap_tavl_find( ld->ld_requests, &needle, ldap_req_cmp );
/* this message */
if ( lr->lr_msgid == msgid ) {
break;
}
}
} }
err = 0; err = 0;
......
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include "../liblber/lber-int.h" #include "../liblber/lber-int.h"
#include "lutil.h" #include "lutil.h"
#include "ldap_avl.h"
#ifdef LDAP_R_COMPILE #ifdef LDAP_R_COMPILE
#include <ldap_pvt_thread.h> #include <ldap_pvt_thread.h>
...@@ -438,7 +439,7 @@ struct ldap_common { ...@@ -438,7 +439,7 @@ struct ldap_common {
/* do not mess with these */ /* do not mess with these */
/* protected by req_mutex */ /* protected by req_mutex */
LDAPRequest *ldc_requests; /* list of outstanding requests */ TAvlnode *ldc_requests; /* list of outstanding requests */
/* protected by res_mutex */ /* protected by res_mutex */
LDAPMessage *ldc_responses; /* list of outstanding responses */ LDAPMessage *ldc_responses; /* list of outstanding responses */
#define ld_requests ldc->ldc_requests #define ld_requests ldc->ldc_requests
...@@ -776,6 +777,8 @@ LDAP_F (LDAPConn *) ldap_new_connection( LDAP *ld, LDAPURLDesc **srvlist, ...@@ -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 ); 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 (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 (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_request( LDAP *ld, LDAPRequest *lr );
LDAP_F (void) ldap_free_connection( LDAP *ld, LDAPConn *lc, int force, int unbind ); 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 ); LDAP_F (void) ldap_dump_connection( LDAP *ld, LDAPConn *lconns, int all );
......
...@@ -574,7 +574,8 @@ ldap_open_internal_connection( LDAP **ldp, ber_socket_t *fdp ) ...@@ -574,7 +574,8 @@ ldap_open_internal_connection( LDAP **ldp, ber_socket_t *fdp )
lr->lr_status = LDAP_REQST_INPROGRESS; lr->lr_status = LDAP_REQST_INPROGRESS;
lr->lr_res_errno = LDAP_SUCCESS; lr->lr_res_errno = LDAP_SUCCESS;
/* no mutex lock needed, we just created this ld here */ /* 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 ); LDAP_MUTEX_LOCK( &ld->ld_conn_mutex );
/* Attach the passed socket as the *LDAP's connection */ /* Attach the passed socket as the *LDAP's connection */
......
...@@ -328,11 +328,16 @@ ldap_send_server_request( ...@@ -328,11 +328,16 @@ ldap_send_server_request(
* request to be in WRITING state. * request to be in WRITING state.
*/ */
rc = 0; rc = 0;
if ( ld->ld_requests && if ( ld->ld_requests != NULL ) {
ld->ld_requests->lr_status == LDAP_REQST_WRITING && TAvlnode *node = ldap_tavl_end( ld->ld_requests, TAVL_DIR_RIGHT );
ldap_int_flush_request( ld, ld->ld_requests ) < 0 ) LDAPRequest *lr;
{
rc = -1; 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 ) { if ( rc ) {
ber_free( ber, 1 ); ber_free( ber, 1 );
...@@ -399,12 +404,8 @@ ldap_send_server_request( ...@@ -399,12 +404,8 @@ ldap_send_server_request(
} }
} }
lr->lr_prev = NULL; rc = ldap_tavl_insert( &ld->ld_requests, lr, ldap_req_cmp, ldap_avl_dup_error );
lr->lr_next = ld->ld_requests; assert( rc == LDAP_SUCCESS );
if ( lr->lr_next != NULL ) {
lr->lr_next->lr_prev = lr;
}
ld->ld_requests = lr;
ld->ld_errno = LDAP_SUCCESS; ld->ld_errno = LDAP_SUCCESS;
if ( ldap_int_flush_request( ld, lr ) == -1 ) { if ( ldap_int_flush_request( ld, lr ) == -1 ) {
...@@ -803,19 +804,10 @@ ldap_free_connection( LDAP *ld, LDAPConn *lc, int force, int unbind ) ...@@ -803,19 +804,10 @@ ldap_free_connection( LDAP *ld, LDAPConn *lc, int force, int unbind )
/* FIXME: is this at all possible? /* FIXME: is this at all possible?
* ldap_ld_free() in unbind.c calls ldap_free_connection() * ldap_ld_free() in unbind.c calls ldap_free_connection()
* with force == 1 __after__ explicitly calling * with force == 1 __after__ explicitly calling
* ldap_free_request() on all requests */ * ldap_tavl_free on ld->ld_requests */
if ( force ) { if ( force ) {
LDAPRequest *lr; ldap_tavl_free( ld->ld_requests, ldap_do_free_request );
ld->ld_requests = NULL;
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;
}
} }
if ( lc->lconn_sb != ld->ld_sb ) { if ( lc->lconn_sb != ld->ld_sb ) {
...@@ -913,17 +905,19 @@ ldap_dump_connection( LDAP *ld, LDAPConn *lconns, int all ) ...@@ -913,17 +905,19 @@ ldap_dump_connection( LDAP *ld, LDAPConn *lconns, int all )
void void
ldap_dump_requests_and_responses( LDAP *ld ) ldap_dump_requests_and_responses( LDAP *ld )
{ {
LDAPRequest *lr;
LDAPMessage *lm, *l; LDAPMessage *lm, *l;
TAvlnode *node;
int i; int i;
Debug1( LDAP_DEBUG_TRACE, "** ld %p Outstanding Requests:\n", Debug1( LDAP_DEBUG_TRACE, "** ld %p Outstanding Requests:\n",
(void *)ld ); (void *)ld );
lr = ld->ld_requests; node = ldap_tavl_end( ld->ld_requests, TAVL_DIR_LEFT );
if ( lr == NULL ) { if ( node == NULL ) {
Debug0( LDAP_DEBUG_TRACE, " Empty\n" ); 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", Debug3( LDAP_DEBUG_TRACE, " * msgid %d, origid %d, status %s\n",
lr->lr_msgid, lr->lr_origid, lr->lr_msgid, lr->lr_origid,
( lr->lr_status == LDAP_REQST_INPROGRESS ) ? "InProgress" : ( lr->lr_status == LDAP_REQST_INPROGRESS ) ? "InProgress" :
...@@ -959,39 +953,21 @@ ldap_dump_requests_and_responses( LDAP *ld ) ...@@ -959,39 +953,21 @@ ldap_dump_requests_and_responses( LDAP *ld )
#endif /* LDAP_DEBUG */ #endif /* LDAP_DEBUG */
/* protected by req_mutex */ /* protected by req_mutex */
static void void
ldap_free_request_int( LDAP *ld, LDAPRequest *lr ) 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 /* if lr_refcnt > 0, the request has been looked up
* by ldap_find_request_by_msgid(); if in the meanwhile * by ldap_find_request_by_msgid(); if in the meanwhile
* the request is free()'d by someone else, just decrease * the request is free()'d by someone else, just decrease
* the reference count and extract it from the request * the reference count; later on, it will be freed. */
* 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;
}
if ( lr->lr_refcnt > 0 ) { if ( lr->lr_refcnt > 0 ) {
assert( lr->lr_refcnt == 1 );
lr->lr_refcnt = -lr->lr_refcnt; lr->lr_refcnt = -lr->lr_refcnt;
lr->lr_prev = NULL;
lr->lr_next = NULL;
return; return;
} }
...@@ -1013,6 +989,29 @@ ldap_free_request_int( LDAP *ld, LDAPRequest *lr ) ...@@ -1013,6 +989,29 @@ ldap_free_request_int( LDAP *ld, LDAPRequest *lr )
LDAP_FREE( 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 */ /* protected by req_mutex */
void void
ldap_free_request( LDAP *ld, LDAPRequest *lr ) ldap_free_request( LDAP *ld, LDAPRequest *lr )
...@@ -1662,19 +1661,24 @@ re_encode_request( LDAP *ld, ...@@ -1662,19 +1661,24 @@ re_encode_request( LDAP *ld,
LDAPRequest * LDAPRequest *
ldap_find_request_by_msgid( LDAP *ld, ber_int_t msgid ) ldap_find_request_by_msgid( LDAP *ld, ber_int_t msgid )
{ {
LDAPRequest *lr; LDAPRequest *lr, needle = {0};
needle.lr_msgid = msgid;
for ( lr = ld->ld_requests; lr != NULL; lr = lr->lr_next ) {
if ( lr->lr_status == LDAP_REQST_COMPLETED ) { lr = ldap_tavl_find( ld->ld_requests, &needle, ldap_req_cmp );
continue; /* Skip completed requests */ if ( lr != NULL && lr->lr_status != LDAP_REQST_COMPLETED ) {
} /* try_read1msg is the only user at the moment and we would free it
if ( msgid == lr->lr_msgid ) { * multiple times if retrieving the request again */
lr->lr_refcnt++; assert( lr->lr_refcnt == 0 );
break; 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 */ /* protected by req_mutex */
...@@ -1683,23 +1687,26 @@ ldap_return_request( LDAP *ld, LDAPRequest *lrx, int freeit ) ...@@ -1683,23 +1687,26 @@ ldap_return_request( LDAP *ld, LDAPRequest *lrx, int freeit )
{ {
LDAPRequest *lr; LDAPRequest *lr;
for ( lr = ld->ld_requests; lr != NULL; lr = lr->lr_next ) { lr = ldap_tavl_find( ld->ld_requests, lrx, ldap_req_cmp );
if ( lr == lrx ) { Debug2( LDAP_DEBUG_TRACE, "ldap_return_request: "
if ( lr->lr_refcnt > 0 ) { "lrx %p, lr %p\n", lrx, lr );
lr->lr_refcnt--; if ( lr ) {
assert( lr == lrx );
} else if ( lr->lr_refcnt < 0 ) { if ( lr->lr_refcnt > 0 ) {
lr->lr_refcnt++; lr->lr_refcnt--;
if ( lr->lr_refcnt == 0 ) { } else if ( lr->lr_refcnt < 0 ) {
lr = NULL; 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 ) { if ( lr == NULL ) {
ldap_free_request_int( ld, lrx ); ldap_free_request_int( ld, lrx );
} else if ( freeit ) { } else if ( freeit ) {
ldap_free_request( ld, lrx ); ldap_free_request( ld, lrx );
} }
......
...@@ -344,13 +344,17 @@ wait4msg( ...@@ -344,13 +344,17 @@ wait4msg(
int serviced = 0; int serviced = 0;
rc = LDAP_MSG_X_KEEP_LOOKING; rc = LDAP_MSG_X_KEEP_LOOKING;
LDAP_MUTEX_LOCK( &ld->ld_req_mutex ); LDAP_MUTEX_LOCK( &ld->ld_req_mutex );
if ( ld->ld_requests && if ( ld->ld_requests != NULL ) {
ld->ld_requests->lr_status == LDAP_REQST_WRITING && TAvlnode *node = ldap_tavl_end( ld->ld_requests, TAVL_DIR_RIGHT );
ldap_is_write_ready( ld, LDAPRequest *lr;
ld->ld_requests->lr_conn->lconn_sb ) )
{ assert( node != NULL );
serviced = 1; lr = node->avl_data;
ldap_int_flush_request( ld, ld->ld_requests ); 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; for ( lc = ld->ld_conns;
rc == LDAP_MSG_X_KEEP_LOOKING && lc != NULL; rc == LDAP_MSG_X_KEEP_LOOKING && lc != NULL;
...@@ -452,7 +456,7 @@ try_read1msg( ...@@ -452,7 +456,7 @@ try_read1msg(
LDAPRequest *lr, *tmplr, dummy_lr = { 0 }; LDAPRequest *lr, *tmplr, dummy_lr = { 0 };
BerElement tmpber; BerElement tmpber;
int rc, refer_cnt, hadref, simple_request, err; int rc, refer_cnt, hadref, simple_request, err;
ber_int_t lderr; ber_int_t lderr = -1;
#ifdef LDAP_CONNECTIONLESS #ifdef LDAP_CONNECTIONLESS
LDAPMessage *tmp = NULL, *chain_head = NULL; LDAPMessage *tmp = NULL, *chain_head = NULL;
......
...@@ -108,9 +108,8 @@ ldap_ld_free( ...@@ -108,9 +108,8 @@ ldap_ld_free(
/* free LDAP structure and outstanding requests/responses */ /* free LDAP structure and outstanding requests/responses */
LDAP_MUTEX_LOCK( &ld->ld_req_mutex ); LDAP_MUTEX_LOCK( &ld->ld_req_mutex );
while ( ld->ld_requests != NULL ) { ldap_tavl_free( ld->ld_requests, ldap_do_free_request );
ldap_free_request( ld, ld->ld_requests ); ld->ld_requests = NULL;
}
LDAP_MUTEX_UNLOCK( &ld->ld_req_mutex ); LDAP_MUTEX_UNLOCK( &ld->ld_req_mutex );
LDAP_MUTEX_LOCK( &ld->ld_conn_mutex ); LDAP_MUTEX_LOCK( &ld->ld_conn_mutex );
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment