From 754a0599516ca7fdb3e20150e57b767397e781b3 Mon Sep 17 00:00:00 2001 From: Pierangelo Masarati Date: Mon, 4 Jul 2005 22:41:54 +0000 Subject: [PATCH] fix concurrency issues --- servers/slapd/back-meta/add.c | 9 +- servers/slapd/back-meta/back-meta.h | 11 +- servers/slapd/back-meta/bind.c | 88 +++++++++++--- servers/slapd/back-meta/compare.c | 11 +- servers/slapd/back-meta/conn.c | 176 +++++++++++++++++++++------- servers/slapd/back-meta/delete.c | 11 +- servers/slapd/back-meta/modify.c | 12 +- servers/slapd/back-meta/modrdn.c | 2 + servers/slapd/back-meta/search.c | 2 + servers/slapd/back-meta/unbind.c | 3 + 10 files changed, 250 insertions(+), 75 deletions(-) diff --git a/servers/slapd/back-meta/add.c b/servers/slapd/back-meta/add.c index a17fdbb24a..31b0fcb59a 100644 --- a/servers/slapd/back-meta/add.c +++ b/servers/slapd/back-meta/add.c @@ -68,7 +68,7 @@ meta_back_add( Operation *op, SlapReply *rs ) if ( ldap_back_dn_massage( &dc, &op->o_req_dn, &mdn ) ) { send_ldap_result( op, rs ); - return rs->sr_err; + goto done; } /* Count number of attributes in entry ( +1 ) */ @@ -187,6 +187,11 @@ retry:; BER_BVZERO( &mdn ); } - return meta_back_op_result( mc, op, rs, candidate ); + (void)meta_back_op_result( mc, op, rs, candidate ); + +done:; + meta_back_release_conn( op, mc ); + + return rs->sr_err; } diff --git a/servers/slapd/back-meta/back-meta.h b/servers/slapd/back-meta/back-meta.h index 9340631b94..c3c23adadb 100644 --- a/servers/slapd/back-meta/back-meta.h +++ b/servers/slapd/back-meta/back-meta.h @@ -271,6 +271,11 @@ meta_back_getconn( int *candidate, ldap_back_send_t sendok ); +extern void +meta_back_release_conn( + Operation *op, + metaconn_t *mc ); + extern int meta_back_retry( Operation *op, @@ -280,7 +285,8 @@ meta_back_retry( ldap_back_send_t sendok ); extern void -meta_back_conn_free( metaconn_t *mc ); +meta_back_conn_free( + metaconn_t *mc ); extern int meta_back_init_one_conn( @@ -304,7 +310,8 @@ meta_back_single_dobind( metaconn_t *msc, int candidate, ldap_back_send_t sendok, - int retries ); + int retries, + int dolock ); extern int meta_back_op_result( diff --git a/servers/slapd/back-meta/bind.c b/servers/slapd/back-meta/bind.c index 120959699f..a3d67e4595 100644 --- a/servers/slapd/back-meta/bind.c +++ b/servers/slapd/back-meta/bind.c @@ -24,6 +24,7 @@ #include +#include #include #include @@ -46,7 +47,7 @@ int meta_back_bind( Operation *op, SlapReply *rs ) { metainfo_t *mi = ( metainfo_t * )op->o_bd->be_private; - metaconn_t *mc; + metaconn_t *mc = NULL; int rc = LDAP_OTHER, i, gotit = 0, isroot = 0; @@ -137,6 +138,8 @@ meta_back_bind( Operation *op, SlapReply *rs ) mc->mc_auth_target = META_BOUND_ALL; } + meta_back_release_conn( op, mc ); + /* * rc is LDAP_SUCCESS if at least one bind succeeded, * err is the last error that occurred during a bind; @@ -257,13 +260,33 @@ retry:; rc = slap_map_api2result( rs ); if ( rs->sr_err == LDAP_UNAVAILABLE && nretries != META_RETRY_NEVER ) { - ldap_unbind_ext_s( msc->msc_ld, NULL, NULL ); - msc->msc_ld = NULL; - msc->msc_bound = 0; +retry_lock:; + switch ( ldap_pvt_thread_mutex_trylock( &mi->mi_conn_mutex ) ) { + case LDAP_PVT_THREAD_EBUSY: + default: + ldap_pvt_thread_yield(); + goto retry_lock; + + case 0: + break; + } + + if ( mc->mc_refcnt == 1 ) { + ldap_unbind_ext_s( msc->msc_ld, NULL, NULL ); + msc->msc_ld = NULL; + msc->msc_bound = 0; + + /* mc here must be the regular mc, reset and ready for init */ + rc = meta_back_init_one_conn( op, rs, mt, msc, + LDAP_BACK_DONTSEND ); + + } else { + /* can't do anything about it */ + rc = 0; + } + + ldap_pvt_thread_mutex_unlock( &mi->mi_conn_mutex ); - /* mc here must be the regular mc, reset and ready for init */ - rc = meta_back_init_one_conn( op, rs, mt, msc, - LDAP_BACK_DONTSEND ); if ( rc ) { if ( nretries > 0 ) { nretries--; @@ -323,7 +346,8 @@ meta_back_single_dobind( metaconn_t *mc, int candidate, ldap_back_send_t sendok, - int nretries ) + int nretries, + int dolock ) { metainfo_t *mi = ( metainfo_t * )op->o_bd->be_private; metatarget_t *mt = &mi->mi_targets[ candidate ]; @@ -406,12 +430,36 @@ retry:; rc = slap_map_api2result( rs ); if ( rc == LDAP_UNAVAILABLE && nretries != META_RETRY_NEVER ) { - ldap_unbind_ext_s( msc->msc_ld, NULL, NULL ); - msc->msc_ld = NULL; - msc->msc_bound = 0; + if ( dolock ) { +retry_lock:; + switch ( ldap_pvt_thread_mutex_trylock( &mi->mi_conn_mutex ) ) { + case LDAP_PVT_THREAD_EBUSY: + default: + ldap_pvt_thread_yield(); + goto retry_lock; + + case 0: + break; + } + } + + if ( mc->mc_refcnt == 1 ) { + ldap_unbind_ext_s( msc->msc_ld, NULL, NULL ); + msc->msc_ld = NULL; + msc->msc_bound = 0; - /* mc here must be the regular mc, reset and ready for init */ - rc = meta_back_init_one_conn( op, rs, mt, msc, LDAP_BACK_DONTSEND ); + /* mc here must be the regular mc, reset and ready for init */ + rc = meta_back_init_one_conn( op, rs, mt, msc, LDAP_BACK_DONTSEND ); + + + } else { + /* can't do anything about it */ + rc = LDAP_UNAVAILABLE; + } + + if ( dolock ) { + ldap_pvt_thread_mutex_unlock( &mi->mi_conn_mutex ); + } if ( rc == LDAP_SUCCESS ) { ldap_pvt_thread_yield(); @@ -501,7 +549,7 @@ meta_back_dobind( } rc = meta_back_single_dobind( op, rs, mc, i, - LDAP_BACK_DONTSEND, mt->mt_nretries ); + LDAP_BACK_DONTSEND, mt->mt_nretries, 1 ); if ( rc != LDAP_SUCCESS ) { rs->sr_err = slap_map_api2result( rs ); @@ -535,11 +583,15 @@ done:; "%s meta_back_dobind: conn=%ld bound=%d\n", op->o_log_prefix, mc->mc_conn->c_connid, bound ); - if ( bound == 0 && ( sendok & LDAP_BACK_SENDERR ) ) { - if ( rs->sr_err == LDAP_SUCCESS ) { - rs->sr_err = LDAP_BUSY; + if ( bound == 0 ) { + meta_back_release_conn( op, mc ); + + if ( sendok & LDAP_BACK_SENDERR ) { + if ( rs->sr_err == LDAP_SUCCESS ) { + rs->sr_err = LDAP_BUSY; + } + send_ldap_result( op, rs ); } - send_ldap_result( op, rs ); } return( bound > 0 ); diff --git a/servers/slapd/back-meta/compare.c b/servers/slapd/back-meta/compare.c index 77701d154e..42a99e59b7 100644 --- a/servers/slapd/back-meta/compare.c +++ b/servers/slapd/back-meta/compare.c @@ -35,7 +35,7 @@ int meta_back_compare( Operation *op, SlapReply *rs ) { metainfo_t *mi = ( metainfo_t * )op->o_bd->be_private; - metaconn_t *mc; + metaconn_t *mc = NULL; char *match = NULL, *err = NULL; struct berval mmatch = BER_BVNULL; @@ -58,7 +58,9 @@ meta_back_compare( Operation *op, SlapReply *rs ) msgid = ch_calloc( sizeof( int ), mi->mi_ntargets ); if ( msgid == NULL ) { - return -1; + send_ldap_error( op, rs, LDAP_OTHER, NULL ); + rc = LDAP_OTHER; + goto done; } /* @@ -314,7 +316,10 @@ finish:; if ( msgid ) { free( msgid ); } - + +done:; + meta_back_release_conn( op, mc ); + return rc; } diff --git a/servers/slapd/back-meta/conn.c b/servers/slapd/back-meta/conn.c index 737dc2e057..1379d8c3a1 100644 --- a/servers/slapd/back-meta/conn.c +++ b/servers/slapd/back-meta/conn.c @@ -157,19 +157,45 @@ metaconn_alloc( * * clears a metaconn */ + void meta_back_conn_free( metaconn_t *mc ) { - if ( mc == NULL ) { - return; - } + assert( mc != NULL ); + assert( mc->mc_refcnt == 0 ); ldap_pvt_thread_mutex_destroy( &mc->mc_mutex ); - free( mc ); } +static void +meta_back_freeconn( + Operation *op, + metaconn_t *mc ) +{ + metainfo_t *mi = ( metainfo_t * )op->o_bd->be_private; + + assert( mc != NULL ); + +retry_lock:; + switch ( ldap_pvt_thread_mutex_trylock( &mi->mi_conn_mutex ) ) { + case LDAP_PVT_THREAD_EBUSY: + default: + ldap_pvt_thread_yield(); + goto retry_lock; + + case 0: + break; + } + + if ( --mc->mc_refcnt == 0 ) { + meta_back_conn_free( mc ); + } + + ldap_pvt_thread_mutex_unlock( &mi->mi_conn_mutex ); +} + /* * meta_back_init_one_conn * @@ -382,24 +408,41 @@ meta_back_retry( { metainfo_t *mi = ( metainfo_t * )op->o_bd->be_private; metatarget_t *mt = &mi->mi_targets[ candidate ]; - int rc; + int rc = LDAP_UNAVAILABLE; metasingleconn_t *msc = &mc->mc_conns[ candidate ]; - ldap_pvt_thread_mutex_lock( &mc->mc_mutex ); +retry_lock:; + switch ( ldap_pvt_thread_mutex_trylock( &mi->mi_conn_mutex ) ) { + case LDAP_PVT_THREAD_EBUSY: + default: + ldap_pvt_thread_yield(); + goto retry_lock; + + case 0: + break; + } - ldap_unbind_ext_s( msc->msc_ld, NULL, NULL ); - msc->msc_ld = NULL; - msc->msc_bound = 0; + assert( mc->mc_refcnt > 0 ); - /* mc here must be the regular mc, reset and ready for init */ - rc = meta_back_init_one_conn( op, rs, mt, msc, sendok ); + if ( mc->mc_refcnt == 1 ) { + ldap_pvt_thread_mutex_lock( &mc->mc_mutex ); - if ( rc == LDAP_SUCCESS ) { - rc = meta_back_single_dobind( op, rs, mc, candidate, - sendok, mt->mt_nretries ); - } + ldap_unbind_ext_s( msc->msc_ld, NULL, NULL ); + msc->msc_ld = NULL; + msc->msc_bound = 0; - ldap_pvt_thread_mutex_unlock( &mc->mc_mutex ); + /* mc here must be the regular mc, reset and ready for init */ + rc = meta_back_init_one_conn( op, rs, mt, msc, sendok ); + + if ( rc == LDAP_SUCCESS ) { + rc = meta_back_single_dobind( op, rs, mc, candidate, + sendok, mt->mt_nretries, 0 ); + } + + ldap_pvt_thread_mutex_unlock( &mc->mc_mutex ); + } + + ldap_pvt_thread_mutex_unlock( &mi->mi_conn_mutex ); return rc == LDAP_SUCCESS ? 1 : 0; } @@ -608,7 +651,8 @@ meta_back_getconn( ldap_back_send_t sendok ) { metainfo_t *mi = ( metainfo_t * )op->o_bd->be_private; - metaconn_t *mc, mc_curr; + metaconn_t *mc = NULL, + mc_curr = { 0 }; int cached = META_TARGET_NONE, i = META_TARGET_NONE, err = LDAP_SUCCESS, @@ -638,6 +682,9 @@ retry_lock:; } mc = (metaconn_t *)avl_find( mi->mi_conntree, (caddr_t)&mc_curr, meta_back_conn_cmp ); + if ( mc ) { + mc->mc_refcnt++; + } ldap_pvt_thread_mutex_unlock( &mi->mi_conn_mutex ); switch ( op->o_tag ) { @@ -686,7 +733,7 @@ retry_lock:; if ( op_type == META_OP_REQUIRE_ALL ) { /* Looks like we didn't get a bind. Open a new session... */ - if ( !mc ) { + if ( mc == NULL ) { mc = metaconn_alloc( op ); mc->mc_conn = op->o_conn; new_conn = 1; @@ -719,7 +766,10 @@ retry_lock:; if ( ncandidates == 0 ) { if ( new_conn ) { - meta_back_conn_free( mc ); + meta_back_freeconn( op, mc ); + + } else { + meta_back_release_conn( op, mc ); } rs->sr_err = LDAP_NO_SUCH_OBJECT; @@ -793,27 +843,34 @@ retry_lock:; "==>meta_back_getconn: got target %d for ndn=\"%s\" from cache\n", i, op->o_req_ndn.bv_val, 0 ); - /* Retries searching for a metaconn in the avl tree */ - mc_curr.mc_conn = op->o_conn; + if ( mc == NULL ) { + /* Retries searching for a metaconn in the avl tree + * the reason is that the connection might have been + * created by meta_back_get_candidate() */ + mc_curr.mc_conn = op->o_conn; retry_lock2:; - switch ( ldap_pvt_thread_mutex_trylock( &mi->mi_conn_mutex ) ) { - case LDAP_PVT_THREAD_EBUSY: - default: - ldap_pvt_thread_yield(); - goto retry_lock2; - - case 0: - break; - } - mc = (metaconn_t *)avl_find( mi->mi_conntree, - (caddr_t)&mc_curr, meta_back_conn_cmp ); - ldap_pvt_thread_mutex_unlock( &mi->mi_conn_mutex ); + switch ( ldap_pvt_thread_mutex_trylock( &mi->mi_conn_mutex ) ) { + case LDAP_PVT_THREAD_EBUSY: + default: + ldap_pvt_thread_yield(); + goto retry_lock2; + + case 0: + break; + } + mc = (metaconn_t *)avl_find( mi->mi_conntree, + (caddr_t)&mc_curr, meta_back_conn_cmp ); + if ( mc != NULL ) { + mc->mc_refcnt++; + } + ldap_pvt_thread_mutex_unlock( &mi->mi_conn_mutex ); - /* Looks like we didn't get a bind. Open a new session... */ - if ( !mc ) { - mc = metaconn_alloc( op ); - mc->mc_conn = op->o_conn; - new_conn = 1; + /* Looks like we didn't get a bind. Open a new session... */ + if ( mc == NULL ) { + mc = metaconn_alloc( op ); + mc->mc_conn = op->o_conn; + new_conn = 1; + } } /* @@ -836,8 +893,11 @@ retry_lock2:; */ candidates[ i ].sr_tag = META_NOT_CANDIDATE; if ( new_conn ) { - ( void )meta_clear_one_candidate( &mc->mc_conns[ i ] ); - meta_back_conn_free( mc ); + (void)meta_clear_one_candidate( &mc->mc_conns[ i ] ); + meta_back_freeconn( op, mc ); + + } else { + meta_back_release_conn( op, mc ); } return NULL; } @@ -855,7 +915,7 @@ retry_lock2:; } else { /* Looks like we didn't get a bind. Open a new session... */ - if ( !mc ) { + if ( mc == NULL ) { mc = metaconn_alloc( op ); mc->mc_conn = op->o_conn; new_conn = 1; @@ -910,7 +970,10 @@ retry_lock2:; if ( ncandidates == 0 ) { if ( new_conn ) { - meta_back_conn_free( mc ); + meta_back_freeconn( op, mc ); + + } else { + meta_back_release_conn( op, mc ); } rs->sr_err = LDAP_NO_SUCH_OBJECT; @@ -960,6 +1023,9 @@ retry_lock3:; /* * Err could be -1 in case a duplicate metaconn is inserted + * + * FIXME: what if the same client issues more than one + * asynchronous operations? */ if ( err != 0 ) { Debug( LDAP_DEBUG_ANY, @@ -968,7 +1034,7 @@ retry_lock3:; rs->sr_err = LDAP_OTHER; rs->sr_text = "Internal server error"; - meta_back_conn_free( mc ); + meta_back_freeconn( op, mc ); if ( sendok & LDAP_BACK_SENDERR ) { send_ldap_result( op, rs ); rs->sr_text = NULL; @@ -989,3 +1055,27 @@ retry_lock3:; return mc; } +void +meta_back_release_conn( + Operation *op, + metaconn_t *mc ) +{ + metainfo_t *mi = ( metainfo_t * )op->o_bd->be_private; + + assert( mc != NULL ); + +retry_lock:; + switch ( ldap_pvt_thread_mutex_trylock( &mi->mi_conn_mutex ) ) { + case LDAP_PVT_THREAD_EBUSY: + default: + ldap_pvt_thread_yield(); + goto retry_lock; + + case 0: + break; + } + + assert( mc->mc_refcnt > 0 ); + mc->mc_refcnt--; + ldap_pvt_thread_mutex_unlock( &mi->mi_conn_mutex ); +} diff --git a/servers/slapd/back-meta/delete.c b/servers/slapd/back-meta/delete.c index 94d9a330d8..d247de359a 100644 --- a/servers/slapd/back-meta/delete.c +++ b/servers/slapd/back-meta/delete.c @@ -35,7 +35,7 @@ int meta_back_delete( Operation *op, SlapReply *rs ) { metainfo_t *mi = ( metainfo_t * )op->o_bd->be_private; - metaconn_t *mc; + metaconn_t *mc = NULL; int candidate = -1; struct berval mdn = BER_BVNULL; dncookie dc; @@ -58,7 +58,7 @@ meta_back_delete( Operation *op, SlapReply *rs ) if ( ldap_back_dn_massage( &dc, &op->o_req_dn, &mdn ) ) { send_ldap_result( op, rs ); - return -1; + goto done; } retry:; @@ -76,6 +76,11 @@ retry:; BER_BVZERO( &mdn ); } - return meta_back_op_result( mc, op, rs, candidate ); + rs->sr_err = meta_back_op_result( mc, op, rs, candidate ); + +done:; + meta_back_release_conn( op, mc ); + + return rs->sr_err; } diff --git a/servers/slapd/back-meta/modify.c b/servers/slapd/back-meta/modify.c index 62a041a17f..2d51289cb1 100644 --- a/servers/slapd/back-meta/modify.c +++ b/servers/slapd/back-meta/modify.c @@ -195,11 +195,15 @@ cleanup:; free( modv ); if ( rc != -1 ) { - return meta_back_op_result( mc, op, rs, candidate ); + rc = meta_back_op_result( mc, op, rs, candidate ); + + } else { + send_ldap_result( op, rs ); + rc = 0; } - - send_ldap_result( op, rs ); - return rs->sr_err; + meta_back_release_conn( op, mc ); + + return rc; } diff --git a/servers/slapd/back-meta/modrdn.c b/servers/slapd/back-meta/modrdn.c index b5396f9b36..a63e99e48c 100644 --- a/servers/slapd/back-meta/modrdn.c +++ b/servers/slapd/back-meta/modrdn.c @@ -131,6 +131,8 @@ cleanup:; send_ldap_result( op, rs ); + meta_back_release_conn( op, mc ); + return rs->sr_err; } diff --git a/servers/slapd/back-meta/search.c b/servers/slapd/back-meta/search.c index 7379141d08..30e0d9ba22 100644 --- a/servers/slapd/back-meta/search.c +++ b/servers/slapd/back-meta/search.c @@ -784,6 +784,8 @@ finish:; } } + meta_back_release_conn( op, mc ); + return rc; } diff --git a/servers/slapd/back-meta/unbind.c b/servers/slapd/back-meta/unbind.c index 5211a9dae3..1fd72e7271 100644 --- a/servers/slapd/back-meta/unbind.c +++ b/servers/slapd/back-meta/unbind.c @@ -57,6 +57,7 @@ retry_lock:; case 0: break; } + mc = avl_delete( &mi->mi_conntree, ( caddr_t )&mc_curr, meta_back_conn_cmp ); ldap_pvt_thread_mutex_unlock( &mi->mi_conn_mutex ); @@ -68,6 +69,8 @@ retry_lock:; "=>meta_back_conn_destroy: destroying conn %ld\n", mc->mc_conn->c_connid, 0, 0 ); + assert( mc->mc_refcnt == 0 ); + /* * Cleanup rewrite session */ -- 2.39.5