ldap/servers/slapd/back-ldbm/ldbm_add.c | 10 ++++++
ldap/servers/slapd/back-ldbm/ldbm_delete.c | 12 +++----
ldap/servers/slapd/back-ldbm/ldbm_modify.c | 38 +++++++++++++++++++++++++
ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 1
4 files changed, 55 insertions(+), 6 deletions(-)
New commits:
commit cef6ae45454ee54f6f97f38eeffe56f46e0648b5
Author: Mark Reynolds <mreynolds(a)redhat.com>
Date: Wed Apr 16 15:21:21 2014 -0400
Ticket 47782 - Parent numbordinate count can be incorrectly updated if an error
occurs
Bug Description: When adding and deleting entries there is a small chance that the
modified
parent entry(numbsubordinates) could be replaced in the entry cache,
even
if the operation fails.
Fix Description: For deletes we can simply move the cache entry switching to a safe
location,
but for adds we need to unswitch the parent's entry in the
cache.
https://fedorahosted.org/389/ticket/47782
Reviewed by: nhosoi(Thanks!)
(cherry picked from commit 2079892800843d1541a8f712d0b1395f75919ee9)
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c
b/ldap/servers/slapd/back-ldbm/ldbm_add.c
index e5b9eeb..3dcceff 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_add.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c
@@ -117,6 +117,7 @@ ldbm_back_add( Slapi_PBlock *pb )
CSN *opcsn = NULL;
entry_address addr = {0};
int not_an_error = 0;
+ int parent_switched = 0;
slapi_pblock_get( pb, SLAPI_PLUGIN_PRIVATE, &li );
slapi_pblock_get( pb, SLAPI_ADD_ENTRY, &e );
@@ -1018,6 +1019,7 @@ ldbm_back_add( Slapi_PBlock *pb )
{
/* switch the parent entry copy into play */
modify_switch_entries( &parent_modify_c,be);
+ parent_switched = 1;
}
if (ruv_c_init) {
@@ -1097,6 +1099,14 @@ error_return:
} else if (0 == rc) {
rc = SLAPI_FAIL_GENERAL;
}
+ if (parent_switched){
+ /*
+ * Restore the old parent entry, switch the new with the original.
+ * Otherwise the numsubordinate count will be off, and could later
+ * be written to disk.
+ */
+ modify_unswitch_entries( &parent_modify_c,be);
+ }
diskfull_return:
if (disk_full) {
rc= return_on_disk_full(li);
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
index 31249c7..792d424 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
@@ -1155,12 +1155,6 @@ ldbm_back_delete( Slapi_PBlock *pb )
CACHE_RETURN(&inst->inst_cache, &e);
e = NULL;
}
-
- if (parent_found)
- {
- /* Replace the old parent entry with the newly modified one */
- modify_switch_entries( &parent_modify_c,be);
- }
if (ruv_c_init) {
if (modify_switch_entries(&ruv_c, be) != 0 ) {
@@ -1172,6 +1166,12 @@ ldbm_back_delete( Slapi_PBlock *pb )
}
}
+ if (parent_found)
+ {
+ /* Replace the old parent entry with the newly modified one */
+ modify_switch_entries( &parent_modify_c,be);
+ }
+
rc= 0;
goto common_return;
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c
b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
index de5dd17..0f01b2f 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
@@ -122,6 +122,44 @@ int modify_switch_entries(modify_context *mc,backend *be)
return ret;
}
+/*
+ * Switch the new with the old(original) - undoing modify_switch_entries()
+ * This expects modify_term() to be called next, as the old "new" entry
+ * is now gone(replaced by the original entry).
+ */
+int
+modify_unswitch_entries(modify_context *mc,backend *be)
+{
+ struct backentry *tmp_be;
+ ldbm_instance *inst = (ldbm_instance *) be->be_instance_info;
+ int ret = 0;
+
+ if (mc->old_entry!=NULL && mc->new_entry!=NULL) {
+ /* switch the entries, and reset the new, new, entry */
+ tmp_be = mc->new_entry;
+ mc->new_entry = mc->old_entry;
+ mc->new_entry->ep_state = 0;
+ mc->new_entry->ep_refcnt = 0;
+ mc->new_entry_in_cache = 0;
+ mc->old_entry = tmp_be;
+
+ ret = cache_replace(&(inst->inst_cache), mc->old_entry, mc->new_entry);
+ if (ret == 0) {
+ /*
+ * The new entry was originally locked, so since we did the
+ * switch we need to unlock the "new" entry, and return the
+ * "old" one. modify_term() will then return the "new" entry.
+ */
+ cache_unlock_entry(&inst->inst_cache, mc->new_entry);
+ CACHE_RETURN( &(inst->inst_cache), &(mc->old_entry) );
+ mc->new_entry_in_cache = 1;
+ mc->old_entry = NULL;
+ }
+ }
+
+ return ret;
+}
+
/* This routine does that part of a modify operation which involves
updating the on-disk data: updates idices, id2entry.
Copes properly with DB_LOCK_DEADLOCK. The caller must be able to cope with
diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
index 6372f50..d41ce1c 100644
--- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
+++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
@@ -361,6 +361,7 @@ void modify_init(modify_context *mc,struct backentry *old_entry);
int modify_apply_mods(modify_context *mc, Slapi_Mods *smods);
int modify_term(modify_context *mc,backend *be);
int modify_switch_entries(modify_context *mc,backend *be);
+int modify_unswitch_entries(modify_context *mc,backend *be);
int modify_apply_mods_ignore_error(modify_context *mc, Slapi_Mods *smods, int error);
/*