ITS#5853 restructure wait4msg / try_read1msg again. Consolidate

the two try_read1msg cases into one, bump refcnts to prevent
lconn's from being freed prematurely.
This commit is contained in:
Howard Chu 2009-02-10 09:51:31 +00:00
parent ab123880df
commit 80c6ea52ea
2 changed files with 69 additions and 99 deletions

View File

@ -957,6 +957,9 @@ ldap_is_read_ready( LDAP *ld, Sockbuf *sb )
sip = (struct selectinfo *)ld->ld_selectinfo;
if (ber_sockbuf_ctrl( sb, LBER_SB_OPT_DATA_READY, NULL ))
return 1;
ber_sockbuf_ctrl( sb, LBER_SB_OPT_GET_FD, &sd );
#ifdef HAVE_POLL

View File

@ -71,7 +71,7 @@ static int ldap_mark_abandoned LDAP_P(( LDAP *ld, ber_int_t msgid, int idx ));
static int wait4msg LDAP_P(( LDAP *ld, ber_int_t msgid, int all, struct timeval *timeout,
LDAPMessage **result ));
static ber_tag_t try_read1msg LDAP_P(( LDAP *ld, ber_int_t msgid,
int all, LDAPConn **lc, LDAPMessage **result ));
int all, LDAPConn *lc, LDAPMessage **result ));
static ber_tag_t build_result_ber LDAP_P(( LDAP *ld, BerElement **bp, LDAPRequest *lr ));
static void merge_error_info LDAP_P(( LDAP *ld, LDAPRequest *parentr, LDAPRequest *lr ));
static LDAPMessage * chkResponseList LDAP_P(( LDAP *ld, int msgid, int all));
@ -106,7 +106,7 @@ ldap_result(
struct timeval *timeout,
LDAPMessage **result )
{
LDAPMessage *lm = NULL;
LDAPMessage *lm;
int rc;
assert( ld != NULL );
@ -118,19 +118,7 @@ ldap_result(
ldap_pvt_thread_mutex_lock( &ld->ld_res_mutex );
#endif
#if 0
/* this is already done inside wait4msg(), right?... */
lm = chkResponseList( ld, msgid, all );
#endif
if ( lm == NULL ) {
rc = wait4msg( ld, msgid, all, timeout, result );
} else {
*result = lm;
ld->ld_errno = LDAP_SUCCESS;
rc = lm->lm_msgtype;
}
rc = wait4msg( ld, msgid, all, timeout, result );
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_unlock( &ld->ld_res_mutex );
@ -335,13 +323,6 @@ wait4msg(
if ( ber_sockbuf_ctrl( lc->lconn_sb,
LBER_SB_OPT_DATA_READY, NULL ) )
{
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_unlock( &ld->ld_conn_mutex );
#endif
rc = try_read1msg( ld, msgid, all, &lc, result );
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex );
#endif
lc_ready = 1;
break;
}
@ -375,55 +356,64 @@ wait4msg(
rc = LDAP_MSG_X_KEEP_LOOKING; /* select interrupted: loop */
} else {
rc = LDAP_MSG_X_KEEP_LOOKING;
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_lock( &ld->ld_req_mutex );
#endif
if ( ld->ld_requests &&
ld->ld_requests->lr_status == LDAP_REQST_WRITING &&
ldap_is_write_ready( ld,
ld->ld_requests->lr_conn->lconn_sb ) )
{
ldap_int_flush_request( ld, ld->ld_requests );
}
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex );
#endif
for ( lc = ld->ld_conns;
rc == LDAP_MSG_X_KEEP_LOOKING && lc != NULL; )
{
if ( lc->lconn_status == LDAP_CONNST_CONNECTED &&
ldap_is_read_ready( ld, lc->lconn_sb ) )
{
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_unlock( &ld->ld_conn_mutex );
#endif
rc = try_read1msg( ld, msgid, all, &lc, result );
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex );
#endif
if ( lc == NULL ) {
/* if lc gets free()'d,
* there's no guarantee
* lc->lconn_next is still
* sane; better restart
* (ITS#4405) */
lc = ld->ld_conns;
/* don't get to next conn! */
break;
}
}
/* next conn */
lc = lc->lconn_next;
}
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_unlock( &ld->ld_conn_mutex );
#endif
lc_ready = 1;
}
}
if ( lc_ready ) {
LDAPConn *lnext;
rc = LDAP_MSG_X_KEEP_LOOKING;
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_lock( &ld->ld_req_mutex );
#endif
if ( ld->ld_requests &&
ld->ld_requests->lr_status == LDAP_REQST_WRITING &&
ldap_is_write_ready( ld,
ld->ld_requests->lr_conn->lconn_sb ) )
{
ldap_int_flush_request( ld, ld->ld_requests );
}
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex );
#endif
for ( lc = ld->ld_conns;
rc == LDAP_MSG_X_KEEP_LOOKING && lc != NULL;
lc = lnext )
{
if ( lc->lconn_status == LDAP_CONNST_CONNECTED &&
ldap_is_read_ready( ld, lc->lconn_sb ) )
{
/* Don't let it get freed out from under us */
++lc->lconn_refcnt;
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_unlock( &ld->ld_conn_mutex );
#endif
rc = try_read1msg( ld, msgid, all, lc, result );
lnext = lc->lconn_next;
/* Only take locks if we're really freeing */
if ( lc->lconn_refcnt <= 1 ) {
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_lock( &ld->ld_req_mutex );
#endif
ldap_free_connection( ld, lc, 0, 1 );
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
#endif
} else {
--lc->lconn_refcnt;
}
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex );
#endif
} else {
lnext = lc->lconn_next;
}
}
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_unlock( &ld->ld_conn_mutex );
#endif
}
}
if ( rc == LDAP_MSG_X_KEEP_LOOKING && tvp != NULL ) {
@ -482,7 +472,7 @@ try_read1msg(
LDAP *ld,
ber_int_t msgid,
int all,
LDAPConn **lcp,
LDAPConn *lc,
LDAPMessage **result )
{
BerElement *ber;
@ -493,7 +483,6 @@ try_read1msg(
ber_len_t len;
int foundit = 0;
LDAPRequest *lr, *tmplr, dummy_lr = { 0 };
LDAPConn *lc;
BerElement tmpber;
int rc, refer_cnt, hadref, simple_request, err;
ber_int_t lderr;
@ -504,8 +493,7 @@ try_read1msg(
#endif
assert( ld != NULL );
assert( lcp != NULL );
assert( *lcp != NULL );
assert( lc != NULL );
#ifdef LDAP_R_COMPILE
LDAP_PVT_THREAD_ASSERT_MUTEX_OWNER( &ld->ld_res_mutex );
@ -514,8 +502,6 @@ try_read1msg(
Debug( LDAP_DEBUG_TRACE, "read1msg: ld %p msgid %d all %d\n",
(void *)ld, msgid, all );
lc = *lcp;
retry:
if ( lc->lconn_ber == NULL ) {
lc->lconn_ber = ldap_alloc_ber_with_options( ld );
@ -561,14 +547,8 @@ nextresp3:
if ( err == EAGAIN ) return LDAP_MSG_X_KEEP_LOOKING;
#endif
ld->ld_errno = LDAP_SERVER_DOWN;
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_lock( &ld->ld_req_mutex );
#endif
ldap_free_connection( ld, lc, 1, 0 );
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
#endif
lc = *lcp = NULL;
--lc->lconn_refcnt;
lc->lconn_status = 0;
return -1;
default:
@ -938,14 +918,8 @@ nextresp2:
* shouldn't necessarily end the connection
*/
if ( lc != NULL && id != 0 ) {
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_lock( &ld->ld_req_mutex );
#endif
ldap_free_connection( ld, lc, 0, 1 );
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
#endif
lc = *lcp = NULL;
--lc->lconn_refcnt;
lc = NULL;
}
}
}
@ -1011,14 +985,7 @@ nextresp2:
/* get rid of the connection... */
if ( lc != NULL ) {
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_lock( &ld->ld_req_mutex );
#endif
ldap_free_connection( ld, lc, 0, 1 );
#ifdef LDAP_R_COMPILE
ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
#endif
lc = *lcp = NULL;
--lc->lconn_refcnt;
}
/* need to return -1, because otherwise