ldap/servers/slapd/back-ldbm/ldbm_delete.c | 8 +++++++- ldap/servers/slapd/back-ldbm/ldbm_modify.c | 4 ++++ ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-)
New commits: commit 5db90314f1d0239b928a35e325b4810d14677c6b Author: Rich Megginson rmeggins@redhat.com Date: Thu Mar 25 12:10:46 2010 -0600
Bug 571677 - Busy replica on consumers when directly deleting a replication conflict
https://bugzilla.redhat.com/show_bug.cgi?id=571677 Resolves: bug 571677 Bug Description: Busy replica on consumers when directly deleting a replication conflict Reviewed by: nhosoi (Thanks!) Branch: Directory_Server_8_2_Branch Fix Description: In some cases, urp fixup operations can be called from the bepreop stage of other operations. The ldbm_back_delete() and ldbm_back_modify() code lock the target entry in the cache. If a bepreop then attempts to operate on the same entry and acquire the lock on the entry, deadlock will occur. The modrdn code does not acquire the cache lock on the target entries before calling the bepreops. The modify and delete code does not acquire the cache lock on the target entries before calling the bepostops. I tried unlocking the target entry before calling the bepreops, then locking the entry just after. This causes the problem to disappear, but I do not know if this will lead to race conditions. The modrdn has been working this way forever, and there are no known race conditions with that code. I think the most robust fix for this issue would be to introduce some sort of semaphore instead of a simple mutex on the cached entry. Then cache_lock_entry would look something like this: if entry->sem == 0 entry->sem++ /* acquire entry */ entry->locking_thread = this_thread else if entry->locking_thread == this_thread entry->sem++ /* increment count on this entry */ else wait_for_sem(entry->sem) /* wait until released */ and cache_unlock_entry would look something like this: entry->sem--; if entry->sem == 0 entry->locking_thread = 0 Platforms tested: RHEL5 x86_64 Flag Day: no Doc impact: no (cherry picked from commit eac3f15f2209719e05640e1576b4273d03bef079)
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index 9cb961c..c1cb1cf 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c @@ -178,6 +178,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) /* Don't call pre-op for Tombstone entries */ if (!delete_tombstone_entry) { + int rc = 0; /* * Some present state information is passed through the PBlock to the * backend pre-op plugin. To ensure a consistent snapshot of this state @@ -191,7 +192,12 @@ ldbm_back_delete( Slapi_PBlock *pb ) goto error_return; } slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code); - if(plugin_call_plugins(pb, SLAPI_PLUGIN_BE_PRE_DELETE_FN)==-1) + /* have to unlock the entry here, in case the bepreop attempts + to modify the same entry == deadlock */ + cache_unlock_entry( &inst->inst_cache, e ); + rc = plugin_call_plugins(pb, SLAPI_PLUGIN_BE_PRE_DELETE_FN); + cache_lock_entry( &inst->inst_cache, e ); + if (rc == -1) { /* * Plugin indicated some kind of failure, diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c index 2b17eee..ecd42c9 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c @@ -294,7 +294,11 @@ ldbm_back_modify( Slapi_PBlock *pb ) /* ec is the entry that our bepreop should get to mess with */ slapi_pblock_set( pb, SLAPI_MODIFY_EXISTING_ENTRY, ec->ep_entry ); slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code); + /* have to unlock the entry here, in case the bepreop attempts + to modify the same entry == deadlock */ + cache_unlock_entry( &inst->inst_cache, e ); plugin_call_plugins(pb, SLAPI_PLUGIN_BE_PRE_MODIFY_FN); + cache_lock_entry( &inst->inst_cache, e ); slapi_pblock_get(pb, SLAPI_RESULT_CODE, &ldap_result_code); /* The Plugin may have messed about with some of the PBlock parameters... ie. mods */ slapi_pblock_get( pb, SLAPI_MODIFY_MODS, &mods ); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index 0dd8eb2..8cb7173 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -876,6 +876,7 @@ common_return: cache_return( &inst->inst_cache, &ec ); }
+ moddn_unlock_and_return_entries(be,&e,&existingentry); /* * The bepostop is called even if the operation fails. */ @@ -889,7 +890,6 @@ common_return: slapi_mods_done(&smods_operation_wsi); slapi_mods_done(&smods_generated); slapi_mods_done(&smods_generated_wsi); - moddn_unlock_and_return_entries(be,&e,&existingentry); slapi_ch_free((void**)&child_entries); slapi_ch_free((void**)&child_entry_copies); if (ldap_result_matcheddn && 0 != strcmp(ldap_result_matcheddn, "NULL"))
389-commits@lists.fedoraproject.org