diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c index acb25e7e8c..c16532fe5f 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_add.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c @@ -100,6 +100,7 @@ ldbm_back_add(Slapi_PBlock *pb) int result_sent = 0; int32_t parent_op = 0; int32_t betxn_callback_fails = 0; /* if a BETXN fails we need to revert entry cache */ + int32_t parent_locked = 0; /* non-zero when parent entry cache lock is held */ struct timespec parent_time; if (slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id) < 0) { @@ -219,8 +220,19 @@ ldbm_back_add(Slapi_PBlock *pb) txn.back_txn_txn = NULL; /* ready to create the child transaction */ for (retry_count = 0; retry_count < RETRY_TIMES; retry_count++) { if (txn.back_txn_txn && (txn.back_txn_txn != parent_txn)) { - /* Don't release SERIAL LOCK */ - dblayer_txn_abort_ext(li, &txn, PR_FALSE); + /* + * Unlock entry cache locks before releasing SERIAL LOCK + * to maintain lock ordering (SERIAL LOCK -> entry locks) + * and prevent AB-BA deadlock on retry. + * Entries stay refcounted in cache, so pointers remain valid. + */ + if (parent_locked && parent_modify_c.old_entry) { + cache_unlock_entry(&inst->inst_cache, parent_modify_c.old_entry); + parent_locked = 0; + } + + /* Release SERIAL LOCK */ + dblayer_txn_abort(be, &txn); noabort = 1; slapi_pblock_set(pb, SLAPI_TXN, parent_txn); /* must duplicate addingentry before returning it to cache, @@ -267,11 +279,19 @@ ldbm_back_add(Slapi_PBlock *pb) } /* dblayer_txn_begin holds SERIAL lock, * which should be outside of locking the entry (find_entry2modify) */ - if (0 == retry_count) { - /* First time, hold SERIAL LOCK */ - retval = dblayer_txn_begin(be, parent_txn, &txn); - noabort = 0; + retval = dblayer_txn_begin(be, parent_txn, &txn); + if (0 != retval) { + if (LDBM_OS_ERR_IS_DISKFULL(retval)) { + disk_full = 1; + ldap_result_code = LDAP_OPERATIONS_ERROR; + goto diskfull_return; + } + ldap_result_code = LDAP_OPERATIONS_ERROR; + goto error_return; + } + noabort = 0; + if (0 == retry_count) { if (!is_tombstone_operation) { rc = slapi_setbit_int(rc, SLAPI_RTN_BIT_FETCH_EXISTING_DN_ENTRY); } @@ -391,24 +411,27 @@ ldbm_back_add(Slapi_PBlock *pb) */ /* JCMREPL - Warning: A Plugin could cause an infinite loop by always returning a result code that requires some action. */ } - } else { - /* Otherwise, no SERIAL LOCK */ - retval = dblayer_txn_begin_ext(li, parent_txn, &txn, PR_FALSE); } - if (0 != retval) { - if (LDBM_OS_ERR_IS_DISKFULL(retval)) { - disk_full = 1; - ldap_result_code = LDAP_OPERATIONS_ERROR; - goto diskfull_return; - } - ldap_result_code = LDAP_OPERATIONS_ERROR; - goto error_return; - } - noabort = 0; /* stash the transaction for plugins */ slapi_pblock_set(pb, SLAPI_TXN, txn.back_txn_txn); + /* Re-lock parent after SERIAL LOCK to maintain lock ordering */ + if (retry_count > 0 && parent_found && parent_modify_c.old_entry) { + int lock_rc = cache_lock_entry(&inst->inst_cache, + parent_modify_c.old_entry); + if (lock_rc != 0) { + slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_add", + "conn=%" PRIu64 " op=%d Failed to re-lock parent " + "entry after deadlock retry (rc=%d)\n", + conn_id, op_id, lock_rc); + ldap_result_code = LDAP_OPERATIONS_ERROR; + retval = -1; + goto error_return; + } + parent_locked = 1; + } + if (0 == retry_count) { /* execute just once */ /* Nothing in this if crause modifies persistent store. * it's called just once. */ @@ -439,6 +462,7 @@ ldbm_back_add(Slapi_PBlock *pb) goto error_return; } modify_init(&parent_modify_c, parententry); + parent_locked = 1; } /* Check if the entry we have been asked to add already exists */ @@ -1445,6 +1469,12 @@ ldbm_back_add(Slapi_PBlock *pb) slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_add", "conn=%" PRIu64 " op=%d modify_term: old_entry=0x%p, new_entry=0x%p\n", conn_id, op_id, parent_modify_c.old_entry, parent_modify_c.new_entry); + /* Parent unlocked during retry but not re-locked (error path). + * Return to cache and NULL out to prevent double-unlock in modify_term. */ + if (!parent_locked && parent_modify_c.old_entry) { + CACHE_RETURN(&(inst->inst_cache), &(parent_modify_c.old_entry)); + parent_modify_c.old_entry = NULL; + } myrc = modify_term(&parent_modify_c, be); done_with_pblock_entry(pb, SLAPI_ADD_EXISTING_DN_ENTRY); done_with_pblock_entry(pb, SLAPI_ADD_EXISTING_UNIQUEID_ENTRY); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index cbfb5bca93..a1b27b57ac 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c @@ -82,6 +82,8 @@ int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from Connection *pb_conn; int32_t parent_op = 0; int32_t betxn_callback_fails = 0; /* if a BETXN fails we need to revert entry cache */ + int32_t e_locked = 0; /* non-zero when entry 'e' cache lock is held */ + int32_t parent_locked = 0; /* non-zero when parent entry cache lock is held */ struct timespec parent_time; if (slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id) < 0) { @@ -198,8 +200,23 @@ int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from if (txn.back_txn_txn && (txn.back_txn_txn != parent_txn)) { /* retry_count > 0 */ Slapi_Entry *ent = NULL; - /* Don't release SERIAL LOCK */ - dblayer_txn_abort_ext(li, &txn, PR_FALSE); + /* + * Unlock entry cache locks before releasing SERIAL LOCK + * to maintain lock ordering (SERIAL LOCK -> entry locks) + * and prevent AB-BA deadlock on retry. + * Entries stay refcounted in cache, so pointers remain valid. + */ + if (e_locked) { + cache_unlock_entry(&inst->inst_cache, e); + e_locked = 0; + } + if (parent_locked && parent_modify_c.old_entry) { + cache_unlock_entry(&inst->inst_cache, parent_modify_c.old_entry); + parent_locked = 0; + } + + /* Release SERIAL LOCK */ + dblayer_txn_abort(be, &txn); slapi_pblock_set(pb, SLAPI_TXN, parent_txn); /* reset original entry */ @@ -257,13 +274,7 @@ int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from } #endif } - if (0 == retry_count) { - /* First time, hold SERIAL LOCK */ - retval = dblayer_txn_begin(be, parent_txn, &txn); - } else { - /* Otherwise, no SERIAL LOCK */ - retval = dblayer_txn_begin_ext(li, parent_txn, &txn, PR_FALSE); - } + retval = dblayer_txn_begin(be, parent_txn, &txn); if (0 != retval) { if (LDBM_OS_ERR_IS_DISKFULL(retval)) disk_full = 1; ldap_result_code= LDAP_OPERATIONS_ERROR; @@ -273,6 +284,39 @@ int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from /* stash the transaction */ slapi_pblock_set(pb, SLAPI_TXN, txn.back_txn_txn); + /* Re-lock entries after SERIAL LOCK to maintain lock ordering */ + if (retry_count > 0) { + if (e) { + int lock_rc = cache_lock_entry(&inst->inst_cache, e); + if (lock_rc != 0) { + slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_delete", + "conn=%" PRIu64 " op=%d Failed to re-lock entry " + "after deadlock retry (rc=%d)\n", + conn_id, op_id, lock_rc); + CACHE_RETURN(&inst->inst_cache, &e); + e = NULL; + ldap_result_code = LDAP_OPERATIONS_ERROR; + retval = -1; + goto error_return; + } + e_locked = 1; + } + if (parent_found && parent_modify_c.old_entry) { + int lock_rc = cache_lock_entry(&inst->inst_cache, + parent_modify_c.old_entry); + if (lock_rc != 0) { + slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_delete", + "conn=%" PRIu64 " op=%d Failed to re-lock parent " + "entry after deadlock retry (rc=%d)\n", + conn_id, op_id, lock_rc); + ldap_result_code = LDAP_OPERATIONS_ERROR; + retval = -1; + goto error_return; + } + parent_locked = 1; + } + } + if (0 == retry_count) { /* just once */ /* find and lock the entry we are about to modify */ /* @@ -291,6 +335,7 @@ int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_delete", "Deleting entry is already deleted.\n"); goto error_return; /* error result sent by find_entry2modify() */ } + e_locked = 1; ep_id = e->ep_id; /* JCMACL - Shouldn't the access check be before the has children check... @@ -356,6 +401,7 @@ int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from CACHE_REMOVE(&inst->inst_cache, e); } cache_unlock_entry(&inst->inst_cache, e); + e_locked = 0; CACHE_RETURN(&inst->inst_cache, &e); /* * e is unlocked and no longer in cache. @@ -540,6 +586,9 @@ int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from * Its possible that the parent entry retrieved from the cache in id2entry * could be removed before we lock it, because tombstone purging updated/replaced * the parent. If we fail to lock the entry, just try again. + * + * Must not sleep while holding SERIAL LOCK -- abort txn + * first, sleep, then re-begin (same as deadlock retry). */ while (1) { parent = id2entry(be, pid, &txn, &retval); @@ -547,10 +596,39 @@ int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from /* Failed to obtain parent entry's entry lock */ if (cache_retry == RETRY_CACHE_LOCK && cache_retry_count < LDBM_CACHE_RETRY_COUNT) { - /* try again */ CACHE_RETURN(&(inst->inst_cache), &parent); + /* Unlock entry, release SERIAL LOCK, sleep, re-acquire */ + cache_unlock_entry(&inst->inst_cache, e); + e_locked = 0; + dblayer_txn_abort(be, &txn); + slapi_pblock_set(pb, SLAPI_TXN, parent_txn); DS_Sleep(PR_MillisecondsToInterval(100)); cache_retry_count++; + txn.back_txn_txn = NULL; + retval = dblayer_txn_begin(be, parent_txn, &txn); + if (0 != retval) { + if (LDBM_OS_ERR_IS_DISKFULL(retval)) + disk_full = 1; + ldap_result_code = LDAP_OPERATIONS_ERROR; + goto error_return; + } + slapi_pblock_set(pb, SLAPI_TXN, txn.back_txn_txn); + /* Re-lock target entry */ + { + int lock_rc = cache_lock_entry(&inst->inst_cache, e); + if (lock_rc != 0) { + slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_delete", + "conn=%" PRIu64 " op=%d Failed to re-lock " + "entry during parent cache lock retry (rc=%d)\n", + conn_id, op_id, lock_rc); + CACHE_RETURN(&inst->inst_cache, &e); + e = NULL; + ldap_result_code = LDAP_OPERATIONS_ERROR; + retval = -1; + goto error_return; + } + e_locked = 1; + } continue; } retval = -1; @@ -576,6 +654,7 @@ int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from int isglue; size_t haschildren = 0; int op = PARENTUPDATE_DEL; + parent_locked = 1; /* Unfortunately findentry doesn't tell us whether it just * didn't find the entry, or if there was an error, so we @@ -1507,7 +1586,10 @@ int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from CACHE_REMOVE(&inst->inst_cache, e); } } - cache_unlock_entry(&inst->inst_cache, e); + if (e_locked) { + cache_unlock_entry(&inst->inst_cache, e); + e_locked = 0; + } CACHE_RETURN(&inst->inst_cache, &e); /* * e is unlocked and no longer in cache. @@ -1549,6 +1631,12 @@ int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from "conn=%" PRIu64 " op=%d modify_term: old_entry=0x%p, new_entry=0x%p, in_cache=%d\n", conn_id, op_id, parent_modify_c.old_entry, parent_modify_c.new_entry, inst ? cache_is_in_cache(&inst->inst_cache, parent_modify_c.new_entry):-1); + /* Parent unlocked during retry but not re-locked (error path). + * Return to cache and NULL out to prevent double-unlock in modify_term. */ + if (!parent_locked && parent_modify_c.old_entry) { + CACHE_RETURN(&(inst->inst_cache), &(parent_modify_c.old_entry)); + parent_modify_c.old_entry = NULL; + } myrc = modify_term(&parent_modify_c, be); if (free_delete_existing_entry) { done_with_pblock_entry(pb, SLAPI_DELETE_EXISTING_ENTRY); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c index c57ba43ae0..765363dbc2 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c @@ -527,6 +527,7 @@ ldbm_back_modify(Slapi_PBlock *pb) int is_noop = 0; int fixup_tombstone = 0; int ec_locked = 0; + int e_locked = 0; int result_sent = 0; int32_t parent_op = 0; int32_t betxn_callback_fails = 0; /* if a BETXN fails we need to revert entry cache */ @@ -620,6 +621,7 @@ ldbm_back_modify(Slapi_PBlock *pb) ldap_result_code = -1; goto error_return; /* error result sent by find_entry2modify() */ } + e_locked = 1; } txn.back_txn_txn = NULL; /* ready to create the child transaction */ @@ -627,8 +629,19 @@ ldbm_back_modify(Slapi_PBlock *pb) int cache_rc = 0; int new_mod_count = 0; if (txn.back_txn_txn && (txn.back_txn_txn != parent_txn)) { - /* don't release SERIAL LOCK */ - dblayer_txn_abort_ext(li, &txn, PR_FALSE); + /* + * Unlock entry cache lock before releasing SERIAL LOCK + * to maintain lock ordering (SERIAL LOCK -> entry locks) + * and prevent AB-BA deadlock on retry. + * Entry stays refcounted in cache, so pointer remains valid. + */ + if (e_locked) { + cache_unlock_entry(&inst->inst_cache, e); + e_locked = 0; + } + + /* Release SERIAL LOCK */ + dblayer_txn_abort(be, &txn); slapi_pblock_set(pb, SLAPI_TXN, parent_txn); /* * Since be_txn_preop functions could have modified the entry/mods, @@ -678,13 +691,7 @@ ldbm_back_modify(Slapi_PBlock *pb) /* Nothing above here modifies persistent store, everything after here is subject to the transaction */ /* dblayer_txn_begin holds SERIAL lock, * which should be outside of locking the entry (find_entry2modify) */ - if (0 == retry_count) { - /* First time, hold SERIAL LOCK */ - retval = dblayer_txn_begin(be, parent_txn, &txn); - } else { - /* Otherwise, no SERIAL LOCK */ - retval = dblayer_txn_begin_ext(li, parent_txn, &txn, PR_FALSE); - } + retval = dblayer_txn_begin(be, parent_txn, &txn); if (0 != retval) { if (LDBM_OS_ERR_IS_DISKFULL(retval)) disk_full = 1; @@ -694,6 +701,22 @@ ldbm_back_modify(Slapi_PBlock *pb) /* stash the transaction for plugins */ slapi_pblock_set(pb, SLAPI_TXN, txn.back_txn_txn); + /* Re-lock entry after SERIAL LOCK to maintain lock ordering */ + if (retry_count > 0 && e) { + int lock_rc = cache_lock_entry(&inst->inst_cache, e); + if (lock_rc != 0) { + slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_modify", + "Failed to re-lock entry after deadlock " + "retry (rc=%d)\n", lock_rc); + CACHE_RETURN(&inst->inst_cache, &e); + e = NULL; + ldap_result_code = LDAP_OPERATIONS_ERROR; + retval = -1; + goto error_return; + } + e_locked = 1; + } + if (0 == retry_count) { /* just once */ if (!MANAGE_ENTRY_BEFORE_DBLOCK(li)) { /* find and lock the entry we are about to modify */ @@ -706,6 +729,7 @@ ldbm_back_modify(Slapi_PBlock *pb) ldap_result_code = -1; goto error_return; /* error result sent by find_entry2modify() */ } + e_locked = 1; } assert(e); @@ -993,6 +1017,7 @@ ldbm_back_modify(Slapi_PBlock *pb) /* we must return both e (which has been deleted) and new entry ec to cache */ /* cache_replace removes e from the cache hash tables */ cache_unlock_entry(&inst->inst_cache, e); + e_locked = 0; CACHE_RETURN(&inst->inst_cache, &e); /* lock new entry in cache to prevent usage until we are complete */ cache_lock_entry(&inst->inst_cache, ec); @@ -1141,7 +1166,10 @@ ldbm_back_modify(Slapi_PBlock *pb) } else if (e) { /* if ec was not in cache, cache_replace was not done. * i.e., e was not unlocked. */ - cache_unlock_entry(&inst->inst_cache, e); + if (e_locked) { + cache_unlock_entry(&inst->inst_cache, e); + e_locked = 0; + } CACHE_RETURN(&inst->inst_cache, &e); } CACHE_RETURN(&inst->inst_cache, &ec); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index e859789b38..e9fa567e8d 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -103,6 +103,9 @@ ldbm_back_modrdn(Slapi_PBlock *pb) int32_t parent_op = 0; int32_t betxn_callback_fails = 0; /* if a BETXN fails we need to revert entry cache */ int32_t cache_mod_phase = 0; /* set when we reach the cache modification phase */ + int32_t e_locked = 0; /* non-zero when entry 'e' cache lock is held */ + int32_t parent_locked = 0; /* non-zero when parententry cache lock is held */ + int32_t newparent_locked = 0; /* non-zero when newparententry cache lock is held */ struct timespec parent_time; Slapi_Mods *smods_add_rdn = NULL; @@ -233,8 +236,27 @@ ldbm_back_modrdn(Slapi_PBlock *pb) if (txn.back_txn_txn && (txn.back_txn_txn != parent_txn)) { Slapi_Entry *ent = NULL; - /* don't release SERIAL LOCK */ - dblayer_txn_abort_ext(li, &txn, PR_FALSE); + /* + * Unlock entry cache locks before releasing SERIAL LOCK + * to maintain lock ordering (SERIAL LOCK -> entry locks) + * and prevent AB-BA deadlock on retry. + * Entries stay refcounted in cache, so pointers remain valid. + */ + if (e_locked) { + cache_unlock_entry(&inst->inst_cache, e); + e_locked = 0; + } + if (parent_locked && parententry) { + cache_unlock_entry(&inst->inst_cache, parententry); + parent_locked = 0; + } + if (newparent_locked && newparententry) { + cache_unlock_entry(&inst->inst_cache, newparententry); + newparent_locked = 0; + } + + /* Release SERIAL LOCK */ + dblayer_txn_abort(be, &txn); /* txn is no longer valid - reset slapi_txn to the parent */ slapi_pblock_set(pb, SLAPI_TXN, parent_txn); @@ -340,13 +362,7 @@ ldbm_back_modrdn(Slapi_PBlock *pb) } #endif } - if (0 == retry_count) { - /* First time, hold SERIAL LOCK */ - retval = dblayer_txn_begin(be, parent_txn, &txn); - } else { - /* Otherwise, no SERIAL LOCK */ - retval = dblayer_txn_begin_ext(li, parent_txn, &txn, PR_FALSE); - } + retval = dblayer_txn_begin(be, parent_txn, &txn); if (0 != retval) { ldap_result_code = LDAP_OPERATIONS_ERROR; if (LDBM_OS_ERR_IS_DISKFULL(retval)) @@ -357,6 +373,51 @@ ldbm_back_modrdn(Slapi_PBlock *pb) /* stash the transaction */ slapi_pblock_set(pb, SLAPI_TXN, (void *)txn.back_txn_txn); + /* Re-lock entries after SERIAL LOCK to maintain lock ordering */ + if (retry_count > 0) { + if (e) { + int lock_rc = cache_lock_entry(&inst->inst_cache, e); + if (lock_rc != 0) { + slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_modrdn", + "conn=%" PRIu64 " op=%d Failed to re-lock entry " + "after deadlock retry (rc=%d)\n", + conn_id, op_id, lock_rc); + CACHE_RETURN(&inst->inst_cache, &e); + e = NULL; + ldap_result_code = LDAP_OPERATIONS_ERROR; + retval = -1; + goto error_return; + } + e_locked = 1; + } + if (parententry) { + int lock_rc = cache_lock_entry(&inst->inst_cache, parententry); + if (lock_rc != 0) { + slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_modrdn", + "conn=%" PRIu64 " op=%d Failed to re-lock parent " + "entry after deadlock retry (rc=%d)\n", + conn_id, op_id, lock_rc); + ldap_result_code = LDAP_OPERATIONS_ERROR; + retval = -1; + goto error_return; + } + parent_locked = 1; + } + if (newparententry) { + int lock_rc = cache_lock_entry(&inst->inst_cache, newparententry); + if (lock_rc != 0) { + slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_modrdn", + "conn=%" PRIu64 " op=%d Failed to re-lock new parent " + "entry after deadlock retry (rc=%d)\n", + conn_id, op_id, lock_rc); + ldap_result_code = LDAP_OPERATIONS_ERROR; + retval = -1; + goto error_return; + } + newparent_locked = 1; + } + } + if (0 == retry_count) { /* just once */ rc = 0; rc = slapi_setbit_int(rc, SLAPI_RTN_BIT_FETCH_EXISTING_DN_ENTRY); @@ -489,6 +550,7 @@ ldbm_back_modrdn(Slapi_PBlock *pb) slapi_pblock_get(pb, SLAPI_RESULT_CODE, &ldap_result_code); goto error_return; /* error result set and sent by find_entry2modify() */ } + e_locked = 1; if (slapi_entry_flag_is_set(e->ep_entry, SLAPI_ENTRY_FLAG_TOMBSTONE) && !is_resurect_operation) { ldap_result_code = LDAP_UNWILLING_TO_PERFORM; @@ -525,6 +587,7 @@ ldbm_back_modrdn(Slapi_PBlock *pb) } modify_init(&parent_modify_context, parententry); + parent_locked = 1; /* Fetch and lock the new parent of the entry that is moving */ if (slapi_sdn_get_ndn(dn_newsuperiordn) != NULL) { @@ -539,6 +602,7 @@ ldbm_back_modrdn(Slapi_PBlock *pb) goto error_return; /* error result set and sent by find_entry2modify() */ } modify_init(&newparent_modify_context, newparententry); + newparent_locked = 1; } opcsn = operation_get_csn(operation); @@ -1448,7 +1512,14 @@ ldbm_back_modrdn(Slapi_PBlock *pb) } } - moddn_unlock_and_return_entry(be, &e); + if (e_locked) { + moddn_unlock_and_return_entry(be, &e); + e_locked = 0; + } else if (e) { + /* Unlocked during retry, not re-locked -- just return to cache */ + CACHE_RETURN(&inst->inst_cache, &e); + e = NULL; + } if (ruv_c_init) { modify_term(&ruv_c, be); @@ -1496,12 +1567,22 @@ ldbm_back_modrdn(Slapi_PBlock *pb) slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_modrdn", "conn=%" PRIu64 " op=%d modify_term: old_entry=0x%p, new_entry=0x%p\n", conn_id, op_id, parent_modify_context.old_entry, parent_modify_context.new_entry); + /* Parent/newparent unlocked during retry but not re-locked (error path). + * Return to cache and NULL out to prevent double-unlock in modify_term. */ + if (!parent_locked && parent_modify_context.old_entry) { + CACHE_RETURN(&(inst->inst_cache), &(parent_modify_context.old_entry)); + parent_modify_context.old_entry = NULL; + } myrc = modify_term(&parent_modify_context, be); slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_modrdn", "conn=%" PRIu64 " op=%d modify_term: rc=%d\n", conn_id, op_id, myrc); slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_modrdn", "conn=%" PRIu64 " op=%d modify_term: old_entry=0x%p, new_entry=0x%p\n", conn_id, op_id, newparent_modify_context.old_entry, newparent_modify_context.new_entry); + if (!newparent_locked && newparent_modify_context.old_entry) { + CACHE_RETURN(&(inst->inst_cache), &(newparent_modify_context.old_entry)); + newparent_modify_context.old_entry = NULL; + } myrc = modify_term(&newparent_modify_context, be); if (free_modrdn_existing_entry) { done_with_pblock_entry(pb, SLAPI_MODRDN_EXISTING_ENTRY);