From 9e811df052522974bcaa8f48d53d5c393bdd378c Mon Sep 17 00:00:00 2001 From: Pierangelo Masarati Date: Wed, 29 Jun 2005 16:38:09 +0000 Subject: [PATCH] seems to definitely fix issues related to ITS#3808 --- servers/slapd/back-ldap/add.c | 5 ++ servers/slapd/back-ldap/back-ldap.h | 1 + servers/slapd/back-ldap/bind.c | 71 +++++++++++++++++++--------- servers/slapd/back-ldap/compare.c | 5 ++ servers/slapd/back-ldap/config.c | 5 ++ servers/slapd/back-ldap/delete.c | 7 ++- servers/slapd/back-ldap/extended.c | 12 ++++- servers/slapd/back-ldap/init.c | 6 ++- servers/slapd/back-ldap/modify.c | 4 ++ servers/slapd/back-ldap/modrdn.c | 6 ++- servers/slapd/back-ldap/proto-ldap.h | 1 + servers/slapd/back-ldap/search.c | 13 +++-- servers/slapd/back-ldap/unbind.c | 6 ++- 13 files changed, 108 insertions(+), 34 deletions(-) diff --git a/servers/slapd/back-ldap/add.c b/servers/slapd/back-ldap/add.c index eb272c7662..e27e6a0255 100644 --- a/servers/slapd/back-ldap/add.c +++ b/servers/slapd/back-ldap/add.c @@ -54,6 +54,7 @@ ldap_back_add( lc = ldap_back_getconn( op, rs, LDAP_BACK_SENDERR ); if ( !lc || !ldap_back_dobind( lc, op, rs, LDAP_BACK_SENDERR ) ) { + lc = NULL; goto cleanup; } @@ -116,6 +117,10 @@ cleanup: ch_free( attrs ); } + if ( lc ) { + ldap_back_release_conn( op, rs, lc ); + } + Debug( LDAP_DEBUG_ARGS, "<== ldap_back_add(\"%s\"): %d\n", op->o_req_dn.bv_val, rs->sr_err, 0 ); diff --git a/servers/slapd/back-ldap/back-ldap.h b/servers/slapd/back-ldap/back-ldap.h index 79b5d8e8e5..ba965d5d85 100644 --- a/servers/slapd/back-ldap/back-ldap.h +++ b/servers/slapd/back-ldap/back-ldap.h @@ -39,6 +39,7 @@ struct ldapconn { int lc_bound; int lc_ispriv; ldap_pvt_thread_mutex_t lc_mutex; + unsigned lc_refcnt; }; /* diff --git a/servers/slapd/back-ldap/bind.c b/servers/slapd/back-ldap/bind.c index 65de40ae64..93aa6abe32 100644 --- a/servers/slapd/back-ldap/bind.c +++ b/servers/slapd/back-ldap/bind.c @@ -101,31 +101,38 @@ done:; /* must re-insert if local DN changed as result of bind */ if ( lc->lc_bound && !dn_match( &op->o_req_ndn, &lc->lc_local_ndn ) ) { - struct ldapconn *tmplc; int lerr; ldap_pvt_thread_mutex_lock( &li->conn_mutex ); - tmplc = avl_delete( &li->conntree, (caddr_t)lc, + /* wait for all other ops to release the connection */ + while ( lc->lc_refcnt > 1 ) { + ldap_pvt_thread_mutex_unlock( &li->conn_mutex ); + ldap_pvt_thread_yield(); + ldap_pvt_thread_mutex_lock( &li->conn_mutex ); + } + assert( lc->lc_refcnt == 1 ); + lc = avl_delete( &li->conntree, (caddr_t)lc, ldap_back_conn_cmp ); - if ( tmplc != NULL ) { - if ( !BER_BVISNULL( &lc->lc_local_ndn ) ) { - ch_free( lc->lc_local_ndn.bv_val ); - } - ber_dupbv( &lc->lc_local_ndn, &op->o_req_ndn ); - lerr = avl_insert( &li->conntree, (caddr_t)lc, - ldap_back_conn_cmp, ldap_back_conn_dup ); + assert( lc != NULL ); - } else { - /* something BAD happened */ - lerr = -1; - rc = LDAP_OTHER; + if ( !BER_BVISNULL( &lc->lc_local_ndn ) ) { + ch_free( lc->lc_local_ndn.bv_val ); } + ber_dupbv( &lc->lc_local_ndn, &op->o_req_ndn ); + lerr = avl_insert( &li->conntree, (caddr_t)lc, + ldap_back_conn_cmp, ldap_back_conn_dup ); ldap_pvt_thread_mutex_unlock( &li->conn_mutex ); if ( lerr == -1 ) { + /* handle this! (e.g. wait until refcnt goes to 1...) */ ldap_back_conn_free( lc ); + lc = NULL; } } + if ( lc != NULL ) { + ldap_back_release_conn( op, rs, lc ); + } + return( rc ); } @@ -223,13 +230,12 @@ ldap_back_freeconn( Operation *op, struct ldapconn *lc ) int rc = 0; ldap_pvt_thread_mutex_lock( &li->conn_mutex ); - lc = avl_delete( &li->conntree, (caddr_t)lc, - ldap_back_conn_cmp ); - if ( lc == NULL ) { - /* something BAD happened */ - rc = -1; + assert( lc->lc_refcnt > 0 ); + if ( --lc->lc_refcnt == 0 ) { + lc = avl_delete( &li->conntree, (caddr_t)lc, + ldap_back_conn_cmp ); + assert( lc != NULL ); - } else { ldap_back_conn_free( (void *)lc ); } ldap_pvt_thread_mutex_unlock( &li->conn_mutex ); @@ -358,6 +364,7 @@ retry:; memset( *lcp, 0, sizeof( struct ldapconn ) ); } (*lcp)->lc_ld = ld; + (*lcp)->lc_refcnt = 1; error_return:; if ( rs->sr_err != LDAP_SUCCESS ) { @@ -407,10 +414,13 @@ ldap_back_getconn( Operation *op, SlapReply *rs, ldap_back_send_t sendok ) ldap_pvt_thread_mutex_lock( &li->conn_mutex ); lc = (struct ldapconn *)avl_find( li->conntree, (caddr_t)&lc_curr, ldap_back_conn_cmp ); + if ( lc != NULL ) { + lc->lc_refcnt++; + } ldap_pvt_thread_mutex_unlock( &li->conn_mutex ); /* Looks like we didn't get a bind. Open a new session... */ - if ( !lc ) { + if ( lc == NULL ) { /* lc here must be NULL */ if ( ldap_back_prepare_conn( &lc, op, rs, sendok ) != LDAP_SUCCESS ) { return NULL; @@ -440,6 +450,7 @@ ldap_back_getconn( Operation *op, SlapReply *rs, ldap_back_send_t sendok ) /* Inserts the newly created ldapconn in the avl tree */ ldap_pvt_thread_mutex_lock( &li->conn_mutex ); + assert( lc->lc_refcnt == 1 ); rs->sr_err = avl_insert( &li->conntree, (caddr_t)lc, ldap_back_conn_cmp, ldap_back_conn_dup ); @@ -450,7 +461,8 @@ ldap_back_getconn( Operation *op, SlapReply *rs, ldap_back_send_t sendok ) ldap_pvt_thread_mutex_unlock( &li->conn_mutex ); Debug( LDAP_DEBUG_TRACE, - "=>ldap_back_getconn: conn %p inserted\n", (void *) lc, 0, 0 ); + "=>ldap_back_getconn: conn %p inserted (refcnt=%u)\n", + (void *)lc, lc->lc_refcnt, 0 ); /* Err could be -1 in case a duplicate ldapconn is inserted */ if ( rs->sr_err != 0 ) { @@ -464,12 +476,27 @@ ldap_back_getconn( Operation *op, SlapReply *rs, ldap_back_send_t sendok ) } } else { Debug( LDAP_DEBUG_TRACE, - "=>ldap_back_getconn: conn %p fetched\n", (void *) lc, 0, 0 ); + "=>ldap_back_getconn: conn %p fetched (refcnt=%u)\n", + (void *)lc, lc->lc_refcnt, 0 ); } return lc; } +void +ldap_back_release_conn( + Operation *op, + SlapReply *rs, + struct ldapconn *lc ) +{ + struct ldapinfo *li = (struct ldapinfo *)op->o_bd->be_private; + + ldap_pvt_thread_mutex_lock( &li->conn_mutex ); + assert( lc->lc_refcnt > 0 ); + lc->lc_refcnt--; + ldap_pvt_thread_mutex_unlock( &li->conn_mutex ); +} + /* * ldap_back_dobind * diff --git a/servers/slapd/back-ldap/compare.c b/servers/slapd/back-ldap/compare.c index a538e3d4b2..44628d98b4 100644 --- a/servers/slapd/back-ldap/compare.c +++ b/servers/slapd/back-ldap/compare.c @@ -44,6 +44,7 @@ ldap_back_compare( lc = ldap_back_getconn( op, rs, LDAP_BACK_SENDERR ); if ( !lc || !ldap_back_dobind( lc, op, rs, LDAP_BACK_SENDERR ) ) { + lc = NULL; goto cleanup; } @@ -70,5 +71,9 @@ retry: cleanup: (void)ldap_back_proxy_authz_ctrl_free( op, &ctrls ); + if ( lc != NULL ) { + ldap_back_release_conn( op, rs, lc ); + } + return rs->sr_err; } diff --git a/servers/slapd/back-ldap/config.c b/servers/slapd/back-ldap/config.c index d01b8a825e..27876cbfec 100644 --- a/servers/slapd/back-ldap/config.c +++ b/servers/slapd/back-ldap/config.c @@ -1573,6 +1573,11 @@ retry: if (rs->sr_err != LDAP_SUCCESS) { rs->sr_err = slap_map_api2result( rs ); } + + if ( lc != NULL ) { + ldap_back_release_conn( &op2, rs, lc ); + } + } else { /* else just do the same as before */ bv = (struct berval *) ch_malloc( sizeof(struct berval) ); diff --git a/servers/slapd/back-ldap/delete.c b/servers/slapd/back-ldap/delete.c index 3d6575be57..08c970388e 100644 --- a/servers/slapd/back-ldap/delete.c +++ b/servers/slapd/back-ldap/delete.c @@ -45,8 +45,7 @@ ldap_back_delete( lc = ldap_back_getconn( op, rs, LDAP_BACK_SENDERR ); if ( !lc || !ldap_back_dobind( lc, op, rs, LDAP_BACK_SENDERR ) ) { - rc = -1; - goto cleanup; + return -1; } ctrls = op->o_ctrls; @@ -71,5 +70,9 @@ retry: cleanup: (void)ldap_back_proxy_authz_ctrl_free( op, &ctrls ); + if ( lc != NULL ) { + ldap_back_release_conn( op, rs, lc ); + } + return rc; } diff --git a/servers/slapd/back-ldap/extended.c b/servers/slapd/back-ldap/extended.c index 8fb7e44a53..070466ae03 100644 --- a/servers/slapd/back-ldap/extended.c +++ b/servers/slapd/back-ldap/extended.c @@ -68,7 +68,8 @@ ldap_back_extended( op->o_ctrls = oldctrls; send_ldap_result( op, rs ); rs->sr_text = NULL; - return rs->sr_err; + rc = rs->sr_err; + goto done; } rc = ( *exop_table[i].extended )( op, rs ); @@ -79,6 +80,11 @@ ldap_back_extended( } op->o_ctrls = oldctrls; +done:; + if ( lc != NULL ) { + ldap_back_release_conn( op, rs, lc ); + } + return rc; } } @@ -170,5 +176,9 @@ retry: rc = -1; } + if ( lc != NULL ) { + ldap_back_release_conn( op, rs, lc ); + } + return rc; } diff --git a/servers/slapd/back-ldap/init.c b/servers/slapd/back-ldap/init.c index 1b8fb45139..b572c36936 100644 --- a/servers/slapd/back-ldap/init.c +++ b/servers/slapd/back-ldap/init.c @@ -202,8 +202,10 @@ void ldap_back_conn_free( void *v_lc ) { struct ldapconn *lc = v_lc; - - ldap_unbind_ext_s( lc->lc_ld, NULL, NULL ); + + if ( lc->lc_ld != NULL ) { + ldap_unbind_ext_s( lc->lc_ld, NULL, NULL ); + } if ( !BER_BVISNULL( &lc->lc_bound_ndn ) ) { ch_free( lc->lc_bound_ndn.bv_val ); } diff --git a/servers/slapd/back-ldap/modify.c b/servers/slapd/back-ldap/modify.c index 0936bb4e0f..5acd9a1aae 100644 --- a/servers/slapd/back-ldap/modify.c +++ b/servers/slapd/back-ldap/modify.c @@ -122,6 +122,10 @@ cleanup:; } ch_free( modv ); + if ( lc != NULL ) { + ldap_back_release_conn( op, rs, lc ); + } + return rc; } diff --git a/servers/slapd/back-ldap/modrdn.c b/servers/slapd/back-ldap/modrdn.c index 22554b2430..437ba646e7 100644 --- a/servers/slapd/back-ldap/modrdn.c +++ b/servers/slapd/back-ldap/modrdn.c @@ -45,7 +45,7 @@ ldap_back_modrdn( lc = ldap_back_getconn( op, rs, LDAP_BACK_SENDERR ); if ( !lc || !ldap_back_dobind( lc, op, rs, LDAP_BACK_SENDERR ) ) { - return( -1 ); + return -1; } if ( op->orr_newSup ) { @@ -78,6 +78,10 @@ retry: cleanup: (void)ldap_back_proxy_authz_ctrl_free( op, &ctrls ); + if ( lc != NULL ) { + ldap_back_release_conn( op, rs, lc ); + } + return rc; } diff --git a/servers/slapd/back-ldap/proto-ldap.h b/servers/slapd/back-ldap/proto-ldap.h index 8c85f507f4..5f29e710a3 100644 --- a/servers/slapd/back-ldap/proto-ldap.h +++ b/servers/slapd/back-ldap/proto-ldap.h @@ -50,6 +50,7 @@ extern BI_entry_get_rw ldap_back_entry_get; int ldap_back_freeconn( Operation *op, struct ldapconn *lc ); struct ldapconn *ldap_back_getconn(struct slap_op *op, struct slap_rep *rs, ldap_back_send_t sendok); +void ldap_back_release_conn( struct slap_op *op, struct slap_rep *rs, struct ldapconn *lc ); int ldap_back_dobind(struct ldapconn *lc, Operation *op, SlapReply *rs, ldap_back_send_t sendok); int ldap_back_retry(struct ldapconn *lc, Operation *op, SlapReply *rs, ldap_back_send_t sendok); int ldap_back_map_result(SlapReply *rs); diff --git a/servers/slapd/back-ldap/search.c b/servers/slapd/back-ldap/search.c index d7fb4a8afb..307e2dbfd6 100644 --- a/servers/slapd/back-ldap/search.c +++ b/servers/slapd/back-ldap/search.c @@ -157,7 +157,7 @@ ldap_back_search( LDAPControl **ctrls = NULL; lc = ldap_back_getconn( op, rs, LDAP_BACK_SENDERR ); - if ( !lc ) { + if ( !lc || !ldap_back_dobind( lc, op, rs, LDAP_BACK_SENDERR ) ) { return rs->sr_err; } @@ -165,9 +165,6 @@ ldap_back_search( * FIXME: in case of values return filter, we might want * to map attrs and maybe rewrite value */ - if ( !ldap_back_dobind( lc, op, rs, LDAP_BACK_SENDERR ) ) { - return rs->sr_err; - } /* should we check return values? */ if ( op->ors_deref != -1 ) { @@ -441,6 +438,10 @@ finish:; ch_free( attrs ); } + if ( lc != NULL ) { + ldap_back_release_conn( op, rs, lc ); + } + return rc; } @@ -722,6 +723,10 @@ cleanup: ch_free( filter ); } + if ( lc != NULL ) { + ldap_back_release_conn( op, &rs, lc ); + } + return rc; } diff --git a/servers/slapd/back-ldap/unbind.c b/servers/slapd/back-ldap/unbind.c index e93dba2c89..c35d2834c2 100644 --- a/servers/slapd/back-ldap/unbind.c +++ b/servers/slapd/back-ldap/unbind.c @@ -53,8 +53,10 @@ ldap_back_conn_destroy( if ( lc ) { Debug( LDAP_DEBUG_TRACE, - "=>ldap_back_conn_destroy: destroying conn %ld\n", - lc->lc_conn->c_connid, 0, 0 ); + "=>ldap_back_conn_destroy: destroying conn %ld (refcnt=%u)\n", + lc->lc_conn->c_connid, lc->lc_refcnt, 0 ); + + assert( lc->lc_refcnt == 0 ); /* * Needs a test because the handler may be corrupted, -- 2.39.5