From 826b177877f78ce6fa8153ff99dd0cf14d801596 Mon Sep 17 00:00:00 2001 From: Howard Chu Date: Fri, 25 Jun 2004 05:29:33 +0000 Subject: [PATCH] More locking cleanup (ITS#3201, etc) --- servers/slapd/back-bdb/cache.c | 1 + servers/slapd/back-bdb/delete.c | 24 ++++++++++---- servers/slapd/back-bdb/dn2entry.c | 12 ++++--- servers/slapd/back-bdb/modrdn.c | 54 +++++++++++++++---------------- 4 files changed, 52 insertions(+), 39 deletions(-) diff --git a/servers/slapd/back-bdb/cache.c b/servers/slapd/back-bdb/cache.c index 8e5dcf995f..85d4d052d4 100644 --- a/servers/slapd/back-bdb/cache.c +++ b/servers/slapd/back-bdb/cache.c @@ -763,6 +763,7 @@ bdb_cache_add( ei.bei_id = e->e_id; ei.bei_parent = eip; ei.bei_nrdn = *nrdn; + ei.bei_lockpad = 0; /* Lock this entry so that bdb_add can run to completion. * It can only fail if BDB has run out of lock resources. diff --git a/servers/slapd/back-bdb/delete.c b/servers/slapd/back-bdb/delete.c index b203488a5f..eca6774136 100644 --- a/servers/slapd/back-bdb/delete.c +++ b/servers/slapd/back-bdb/delete.c @@ -71,6 +71,10 @@ retry: /* transaction retry */ bdb_unlocked_cache_return_entry_w(&bdb->bi_cache, e); e = NULL; } + if( p != NULL ) { + bdb_unlocked_cache_return_entry_r(&bdb->bi_cache, p); + p = NULL; + } #ifdef NEW_LOGGING LDAP_LOG ( OPERATION, DETAIL1, "==> bdb_delete: retrying...\n", 0, 0, 0 ); @@ -201,7 +205,17 @@ retry: /* transaction retry */ goto done; } - bdb_cache_find_id( op, ltid, eip->bei_id, &eip, 0, locker, &plock ); + rc = bdb_cache_find_id( op, ltid, eip->bei_id, &eip, 0, locker, &plock ); + if ( rc ) { + switch( rc ) { + case DB_LOCK_DEADLOCK: + case DB_LOCK_NOTGRANTED: + goto retry; + } + rs->sr_err = LDAP_OTHER; + rs->sr_text = "internal error"; + goto return_results; + } if ( eip ) p = eip->bei_e; if ( pdn.bv_len != 0 ) { @@ -223,9 +237,6 @@ retry: /* transaction retry */ rs->sr_err = access_allowed( op, p, children, NULL, ACL_WRITE, NULL ); - bdb_unlocked_cache_return_entry_r(&bdb->bi_cache, p); - p = NULL; - if ( !rs->sr_err ) { switch( opinfo.boi_err ) { case DB_LOCK_DEADLOCK: @@ -487,8 +498,6 @@ retry: /* transaction retry */ goto return_results; } - bdb_cache_find_id( op, lt2, eip->bei_id, &eip, 0, locker, &plock ); - if ( eip ) p = eip->bei_e; if ( pdn.bv_len != 0 ) { parent_is_glue = is_entry_glue(p); rs->sr_err = bdb_cache_children( op, lt2, p ); @@ -627,6 +636,9 @@ return_results: } done: + if ( p ) + bdb_unlocked_cache_return_entry_r(&bdb->bi_cache, p); + /* free entry */ if( e != NULL ) { if ( rs->sr_err == LDAP_SUCCESS ) { diff --git a/servers/slapd/back-bdb/dn2entry.c b/servers/slapd/back-bdb/dn2entry.c index 25d79e5256..c221edaa08 100644 --- a/servers/slapd/back-bdb/dn2entry.c +++ b/servers/slapd/back-bdb/dn2entry.c @@ -38,7 +38,7 @@ bdb_dn2entry( DB_LOCK *lock ) { EntryInfo *ei = NULL; - int rc; + int rc, rc2; #ifdef NEW_LOGGING LDAP_LOG ( CACHE, ARGS, "bdb_dn2entry(\"%s\")\n", dn->bv_val, 0, 0 ); @@ -56,10 +56,11 @@ bdb_dn2entry( * or not. */ *e = ei; - if ( ei && ei->bei_id ) - bdb_cache_find_id( op, tid, ei->bei_id, + if ( ei && ei->bei_id ) { + rc2 = bdb_cache_find_id( op, tid, ei->bei_id, &ei, 1, locker, lock ); - else if ( ei ) + if ( rc2 ) rc = rc2; + } else if ( ei ) bdb_cache_entryinfo_unlock( ei ); } else if ( ei ) { bdb_cache_entryinfo_unlock( ei ); @@ -72,8 +73,9 @@ bdb_dn2entry( } else if ( matched && rc == DB_NOTFOUND ) { /* always return EntryInfo */ ei = ei->bei_parent; - bdb_cache_find_id( op, tid, ei->bei_id, &ei, 1, + rc2 = bdb_cache_find_id( op, tid, ei->bei_id, &ei, 1, locker, lock ); + if ( rc2 ) rc = rc2; *e = ei; } } diff --git a/servers/slapd/back-bdb/modrdn.c b/servers/slapd/back-bdb/modrdn.c index 2904d1865f..11651e0b28 100644 --- a/servers/slapd/back-bdb/modrdn.c +++ b/servers/slapd/back-bdb/modrdn.c @@ -947,37 +947,35 @@ retry: /* transaction retry */ goto return_results; } - bdb_cache_find_id( op, lt2, eip->bei_id, &eip, 0, locker, &plock ); - if ( eip ) p = eip->bei_e; - if ( p_ndn.bv_len != 0 ) { - parent_is_glue = is_entry_glue(p); - rs->sr_err = bdb_cache_children( op, lt2, p ); - if ( rs->sr_err != DB_NOTFOUND ) { - switch( rs->sr_err ) { - case DB_LOCK_DEADLOCK: - case DB_LOCK_NOTGRANTED: - goto retry; - case 0: - break; - default: + if ( p_ndn.bv_len != 0 ) { + parent_is_glue = is_entry_glue(p); + rs->sr_err = bdb_cache_children( op, lt2, p ); + if ( rs->sr_err != DB_NOTFOUND ) { + switch( rs->sr_err ) { + case DB_LOCK_DEADLOCK: + case DB_LOCK_NOTGRANTED: + goto retry; + case 0: + break; + default: #ifdef NEW_LOGGING - LDAP_LOG ( OPERATION, ERR, - "<=- bdb_modrdn: has_children failed %s (%d)\n", - db_strerror(rs->sr_err), rs->sr_err, 0 ); + LDAP_LOG ( OPERATION, ERR, + "<=- bdb_modrdn: has_children failed %s (%d)\n", + db_strerror(rs->sr_err), rs->sr_err, 0 ); #else - Debug(LDAP_DEBUG_ARGS, - "<=- bdb_modrdn: has_children failed: %s (%d)\n", - db_strerror(rs->sr_err), rs->sr_err, 0 ); + Debug(LDAP_DEBUG_ARGS, + "<=- bdb_modrdn: has_children failed: %s (%d)\n", + db_strerror(rs->sr_err), rs->sr_err, 0 ); #endif - rs->sr_err = LDAP_OTHER; - rs->sr_text = "internal error"; - goto return_results; - } - parent_is_leaf = 1; - } - bdb_unlocked_cache_return_entry_r(&bdb->bi_cache, p); - p = NULL; - } + rs->sr_err = LDAP_OTHER; + rs->sr_text = "internal error"; + goto return_results; + } + parent_is_leaf = 1; + } + bdb_unlocked_cache_return_entry_r(&bdb->bi_cache, p); + p = NULL; + } if ( TXN_COMMIT( lt2, 0 ) != 0 ) { rs->sr_err = LDAP_OTHER;