From: Pierangelo Masarati Date: Sun, 3 Sep 2006 11:04:27 +0000 (+0000) Subject: don't abandon binds in progress; rather unbind (ITS#4663). X-Git-Tag: OPENLDAP_REL_ENG_2_3_MP~218 X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=906ff0d51a7a90276fa1ad2844dd7479c389bd10;p=openldap don't abandon binds in progress; rather unbind (ITS#4663). Better logging for tracing --- diff --git a/servers/slapd/back-meta/bind.c b/servers/slapd/back-meta/bind.c index b82ecd9ed5..ca85a9202c 100644 --- a/servers/slapd/back-meta/bind.c +++ b/servers/slapd/back-meta/bind.c @@ -314,6 +314,10 @@ meta_back_bind_op_result( int nretries = mt->mt_nretries; char buf[ SLAP_TEXT_BUFLEN ]; + Debug( LDAP_DEBUG_TRACE, + ">>> %s meta_back_bind_op_result[%d]\n", + op->o_log_prefix, candidate, 0 ); + if ( rs->sr_err == LDAP_SUCCESS ) { LDAP_BACK_TV_SET( &tv ); @@ -321,7 +325,8 @@ meta_back_bind_op_result( * handle response!!! */ retry:; - switch ( ldap_result( msc->msc_ld, msgid, LDAP_MSG_ALL, &tv, &res ) ) { + rc = ldap_result( msc->msc_ld, msgid, LDAP_MSG_ALL, &tv, &res ); + switch ( rc ) { case 0: Debug( LDAP_DEBUG_ANY, "%s meta_back_bind_op_result[%d]: ldap_result=0 nretries=%d.\n", @@ -336,8 +341,16 @@ retry:; goto retry; } + /* FIXME: binds cannot be abandoned */ + /* don't let anyone else use this handler, + * because there's a pending bind that will not + * be acknowledged */ + ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex ); + ldap_unbind_ext( msc->msc_ld, NULL, NULL ); + msc->msc_ld = NULL; rs->sr_err = LDAP_BUSY; - (void)meta_back_cancel( mc, op, rs, msgid, candidate, sendok ); + LDAP_BACK_CONN_BINDING_CLEAR( msc ); + ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); break; case -1: @@ -368,7 +381,13 @@ retry:; } } - return rs->sr_err = slap_map_api2result( rs ); + rs->sr_err = slap_map_api2result( rs ); + + Debug( LDAP_DEBUG_TRACE, + "<<< %s meta_back_bind_op_result[%d] err=%d\n", + op->o_log_prefix, candidate, rs->sr_err ); + + return rs->sr_err; } /* @@ -787,23 +806,29 @@ meta_back_cancel( metatarget_t *mt = mi->mi_targets[ candidate ]; metasingleconn_t *msc = &mc->mc_conns[ candidate ]; + int rc = LDAP_OTHER; + + Debug( LDAP_DEBUG_TRACE, ">>> %s meta_back_cancel[%d] msgid=%d\n", + op->o_log_prefix, candidate, msgid ); + /* default behavior */ if ( META_BACK_TGT_ABANDON( mt ) ) { - return ldap_abandon_ext( msc->msc_ld, msgid, NULL, NULL ); - } + rc = ldap_abandon_ext( msc->msc_ld, msgid, NULL, NULL ); - if ( META_BACK_TGT_IGNORE( mt ) ) { - return LDAP_SUCCESS; - } + } else if ( META_BACK_TGT_IGNORE( mt ) ) { + rc = LDAP_SUCCESS; + + } else if ( META_BACK_TGT_CANCEL( mt ) ) { + rc = ldap_cancel_s( msc->msc_ld, msgid, NULL, NULL ); - if ( META_BACK_TGT_CANCEL( mt ) ) { - /* FIXME: asynchronous? */ - return ldap_cancel_s( msc->msc_ld, msgid, NULL, NULL ); + } else { + assert( 0 ); } - assert( 0 ); + Debug( LDAP_DEBUG_TRACE, "<<< %s meta_back_cancel[%d] err=%d\n", + op->o_log_prefix, candidate, rc ); - return LDAP_OTHER; + return rc; } diff --git a/servers/slapd/back-meta/conn.c b/servers/slapd/back-meta/conn.c index 99a03636ee..99a7899484 100644 --- a/servers/slapd/back-meta/conn.c +++ b/servers/slapd/back-meta/conn.c @@ -547,9 +547,10 @@ meta_back_retry( metaconn_t *mc = *mcp; metasingleconn_t *msc = &mc->mc_conns[ candidate ]; int rc = LDAP_UNAVAILABLE, - binding = LDAP_BACK_CONN_BINDING( msc ); + binding; ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex ); + binding = LDAP_BACK_CONN_BINDING( msc ); assert( mc->mc_refcnt > 0 ); if ( mc->mc_refcnt == 1 ) { diff --git a/servers/slapd/back-meta/search.c b/servers/slapd/back-meta/search.c index 8bd5877f78..caadd34495 100644 --- a/servers/slapd/back-meta/search.c +++ b/servers/slapd/back-meta/search.c @@ -93,20 +93,25 @@ meta_search_dobind_init( return META_SEARCH_CANDIDATE; } + retcode = META_SEARCH_BINDING; ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex ); if ( LDAP_BACK_CONN_ISBOUND( msc ) || LDAP_BACK_CONN_ISANON( msc ) ) { - ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); - return META_SEARCH_CANDIDATE; - } + /* already bound (or anonymous) */ + retcode = META_SEARCH_CANDIDATE; - if ( LDAP_BACK_CONN_BINDING( msc ) ) { - ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); + } else if ( LDAP_BACK_CONN_BINDING( msc ) ) { + /* another thread is binding the target for this conn; wait */ candidates[ candidate ].sr_msgid = META_MSGID_NEED_BIND; - return META_SEARCH_NEED_BIND; - } + retcode = META_SEARCH_NEED_BIND; - LDAP_BACK_CONN_BINDING_SET( msc ); + } else { + /* we'll need to bind the target for this conn */ + LDAP_BACK_CONN_BINDING_SET( msc ); + } ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); + if ( retcode != META_SEARCH_BINDING ) { + return retcode; + } /* NOTE: this obsoletes pseudorootdn */ if ( op->o_conn != NULL && @@ -130,7 +135,8 @@ meta_search_dobind_init( } if ( LDAP_BACK_CONN_ISBOUND( msc ) ) { - /* idassert ws configured with SASL bind */ + /* apparently, idassert was configured with SASL bind, + * so bind occurred inside meta_back_proxy_authz_cred() */ ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex ); LDAP_BACK_CONN_BINDING_CLEAR( msc ); ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); @@ -161,6 +167,8 @@ meta_search_dobind_init( down:; case LDAP_SERVER_DOWN: + /* This is the worst thing that could happen: + * the search will wait until the retry is over. */ if ( meta_back_retry( op, rs, mcp, candidate, LDAP_BACK_DONTSEND ) ) { return META_SEARCH_CANDIDATE; } @@ -234,6 +242,7 @@ meta_search_dobind_result( } } else { + /* FIXME: check if bound as idassert authcDN! */ if ( be_isroot( op ) ) { LDAP_BACK_CONN_ISBOUND_SET( msc ); } else { @@ -309,7 +318,8 @@ meta_back_search_start( /* * this target is no longer candidate */ - return META_SEARCH_NOT_CANDIDATE; + retcode = META_SEARCH_NOT_CANDIDATE; + goto doreturn; } break; @@ -344,7 +354,8 @@ meta_back_search_start( /* * this target is no longer candidate */ - return META_SEARCH_NOT_CANDIDATE; + retcode = META_SEARCH_NOT_CANDIDATE; + goto doreturn; } } @@ -354,7 +365,7 @@ meta_back_search_start( Debug( LDAP_DEBUG_TRACE, "%s <<< meta_search_dobind_init[%d]=%d\n", op->o_log_prefix, candidate, retcode ); if ( retcode != META_SEARCH_CANDIDATE ) { - return retcode; + goto doreturn; } /* @@ -370,14 +381,16 @@ meta_back_search_start( rs->sr_err = LDAP_UNWILLING_TO_PERFORM; rs->sr_text = "Operation not allowed"; send_ldap_result( op, rs ); - return META_SEARCH_ERR; + retcode = META_SEARCH_ERR; + goto doreturn; case REWRITE_REGEXEC_ERR: /* * this target is no longer candidate */ - return META_SEARCH_NOT_CANDIDATE; + retcode = META_SEARCH_NOT_CANDIDATE; + goto doreturn; } /* @@ -426,7 +439,8 @@ meta_back_search_start( ctrls = op->o_ctrls; if ( ldap_back_proxy_authz_ctrl( &msc->msc_bound_ndn, - mt->mt_version, &mt->mt_idassert, op, rs, &ctrls ) != LDAP_SUCCESS ) + mt->mt_version, &mt->mt_idassert, op, rs, &ctrls ) + != LDAP_SUCCESS ) { candidates[ candidate ].sr_msgid = META_MSGID_IGNORE; retcode = META_SEARCH_NOT_CANDIDATE; @@ -478,6 +492,9 @@ done:; free( mbase.bv_val ); } +doreturn:; + Debug( LDAP_DEBUG_TRACE, "%s <<< meta_back_search_start[%d]=%d\n", op->o_log_prefix, candidate, retcode ); + return retcode; } @@ -649,9 +666,15 @@ meta_back_search( Operation *op, SlapReply *rs ) } if ( candidates[ i ].sr_msgid == META_MSGID_NEED_BIND ) { + meta_search_candidate_t retcode; + /* initiate dobind */ - switch ( meta_search_dobind_init( op, rs, &mc, i, candidates ) ) - { + retcode = meta_search_dobind_init( op, rs, &mc, i, candidates ); + + Debug( LDAP_DEBUG_TRACE, "%s <<< meta_search_dobind_init[%d]=%d\n", + op->o_log_prefix, i, retcode ); + + switch ( retcode ) { case META_SEARCH_BINDING: case META_SEARCH_NEED_BIND: break; @@ -1127,11 +1150,24 @@ really_bad:; /* check for abandon */ if ( op->o_abandon || doabandon ) { for ( i = 0; i < mi->mi_ntargets; i++ ) { - if ( candidates[ i ].sr_msgid != META_MSGID_IGNORE ) - { - (void)meta_back_cancel( mc, op, rs, - candidates[ i ].sr_msgid, i, - LDAP_BACK_DONTSEND ); + if ( candidates[ i ].sr_msgid >= 0 ) { + if ( META_IS_BINDING( &candidates[ i ] ) ) { + ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex ); + if ( LDAP_BACK_CONN_BINDING( &mc->mc_conns[ i ] ) ) { + /* if still binding, destroy */ + ldap_unbind_ext( mc->mc_conns[ i ].msc_ld, NULL, NULL ); + mc->mc_conns[ i ].msc_ld = NULL; + LDAP_BACK_CONN_BINDING_CLEAR( &mc->mc_conns[ i ] ); + } + ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); + META_BINDING_CLEAR( &candidates[ i ] ); + + } else { + (void)meta_back_cancel( mc, op, rs, + candidates[ i ].sr_msgid, i, + LDAP_BACK_DONTSEND ); + } + candidates[ i ].sr_msgid = META_MSGID_IGNORE; } } @@ -1293,6 +1329,13 @@ finish:; ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex ); if ( LDAP_BACK_CONN_BINDING( &mc->mc_conns[ i ] ) ) { LDAP_BACK_CONN_BINDING_CLEAR( &mc->mc_conns[ i ] ); + + assert( candidates[ i ].sr_msgid >= 0 ); + assert( mc->mc_conns[ i ].msc_ld != NULL ); + + /* if still binding, destroy */ + ldap_unbind_ext( mc->mc_conns[ i ].msc_ld, NULL, NULL ); + mc->mc_conns[ i ].msc_ld = NULL; } ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); META_BINDING_CLEAR( &candidates[ i ] ); @@ -1327,7 +1370,9 @@ finish:; * NOTE: should we quarantine the target as well? right now, the connection * is invalidated; the next time it will be recreated and the target * will be quarantined if it cannot be contacted */ - if ( mi->mi_idle_timeout != 0 && rs->sr_err == LDAP_TIMELIMIT_EXCEEDED && op->o_time > mc->mc_conns[ i ].msc_time ) + if ( mi->mi_idle_timeout != 0 + && rs->sr_err == LDAP_TIMELIMIT_EXCEEDED + && op->o_time > mc->mc_conns[ i ].msc_time ) { /* don't let anyone else use this expired connection */ LDAP_BACK_CONN_TAINTED_SET( mc ); @@ -1435,7 +1480,7 @@ meta_send_entry( mapped.bv_val, text ); Debug( LDAP_DEBUG_ANY, "%s", buf, 0, 0 ); - ch_free( attr ); + attr_free( attr ); continue; } } @@ -1458,7 +1503,7 @@ meta_send_entry( */ ( void )ber_scanf( &ber, "x" /* [W] */ ); - ch_free(attr); + attr_free(attr); continue; }