[389-commits] Branch '389-ds-base-1.2.10' - ldap/servers
Richard Allen Megginson
rmeggins at fedoraproject.org
Thu May 24 20:43:25 UTC 2012
ldap/servers/plugins/usn/usn.c | 11 -------
ldap/servers/slapd/back-ldbm/ldbm_delete.c | 42 ++++++-----------------------
2 files changed, 10 insertions(+), 43 deletions(-)
New commits:
commit cc4095d2eca31608afd6c0d04e7c3f45d6a7ae49
Author: Rich Megginson <rmeggins at redhat.com>
Date: Tue May 22 19:13:47 2012 -0600
Ticket #382 - DS Shuts down intermittently
https://fedorahosted.org/389/ticket/382
Resolves: Ticket #382
Bug Description: DS Shuts down intermittently
Reviewed by: nhosoi (Thanks!)
Branch: 389-ds-base-1.2.10
Fix Description: The usn code called from ldbm_back_delete was adding
entryusn and preventryusn to the entry directly in the cache. If another
thread was accessing the entry (e.g. an internal search done by referint or
memberof, or an external search) at the same time, the server would crash.
The fix is to not modify the cached entry directly - instead, modify a
copy of the entry, and apply the entryusn set in the copy to the tombstone.
Then we don't need to use SLAPI_DELETE_BEPREOP_ENTRY or
SLAPI_DELETE_BEPOSTOP_ENTRY at all. We don't need to clean up the
postentry - we are not setting entryusn or preventryusn in the real entry -
we are setting those values in the tombstone entry - if successful, the
values will be added or deleted accordingly - if not successful, the
values are thrown away and freed.
Platforms tested: RHEL6 x86_64
Flag Day: no
Doc impact: no
diff --git a/ldap/servers/plugins/usn/usn.c b/ldap/servers/plugins/usn/usn.c
index c8672d5..da85f7d 100644
--- a/ldap/servers/plugins/usn/usn.c
+++ b/ldap/servers/plugins/usn/usn.c
@@ -429,7 +429,7 @@ usn_bepreop_delete(Slapi_PBlock *pb)
"--> usn_bepreop_delete\n");
/* add next USN to the entry; "be" contains the usn counter */
- slapi_pblock_get(pb, SLAPI_DELETE_BEPREOP_ENTRY, &e);
+ slapi_pblock_get(pb, SLAPI_DELETE_EXISTING_ENTRY, &e);
if (NULL == e) {
rc = LDAP_NO_SUCH_OBJECT;
goto bail;
@@ -576,15 +576,6 @@ usn_bepostop_delete (Slapi_PBlock *pb)
/* if op is not successful, don't increment the counter */
slapi_pblock_get(pb, SLAPI_RESULT_CODE, &rc);
if (LDAP_SUCCESS != rc) {
- Slapi_Entry *e = NULL;
-
- slapi_pblock_get(pb, SLAPI_DELETE_BEPOSTOP_ENTRY, &e);
- if (NULL == e) {
- rc = LDAP_NO_SUCH_OBJECT;
- goto bail;
- }
- /* okay to return the rc from slapi_entry_delete_values */
- rc = slapi_entry_delete_values(e, SLAPI_ATTR_ENTRYUSN_PREV, NULL);
goto bail;
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
index 014af73..865ae2f 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
@@ -94,7 +94,6 @@ ldbm_back_delete( Slapi_PBlock *pb )
int addordel_flags = 0; /* passed to index_addordel */
char *entryusn_str = NULL;
char *prev_entryusn_str = NULL;
- Slapi_Entry *orig_entry = NULL;
Slapi_DN parentsdn;
slapi_pblock_get( pb, SLAPI_BACKEND, &be);
@@ -187,10 +186,6 @@ ldbm_back_delete( Slapi_PBlock *pb )
goto error_return;
}
- /* set entry in case be-preop plugins need to work on it (e.g., USN) */
- slapi_pblock_get( pb, SLAPI_DELETE_BEPREOP_ENTRY, &orig_entry );
- slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, e->ep_entry );
-
/* Don't call pre-op for Tombstone entries */
if (!delete_tombstone_entry)
{
@@ -205,8 +200,6 @@ ldbm_back_delete( Slapi_PBlock *pb )
if(ldap_result_code==LDAP_OPERATIONS_ERROR ||
ldap_result_code==LDAP_INVALID_DN_SYNTAX)
{
- /* restore original entry so the front-end delete code can free it */
- slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry );
/* retval is -1 */
goto error_return;
}
@@ -221,8 +214,6 @@ ldbm_back_delete( Slapi_PBlock *pb )
* or that this Operation became a No-Op.
*/
slapi_pblock_get(pb, SLAPI_RESULT_CODE, &ldap_result_code);
- /* restore original entry so the front-end delete code can free it */
- slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry );
/* retval is -1 */
goto error_return;
}
@@ -231,8 +222,6 @@ ldbm_back_delete( Slapi_PBlock *pb )
OP_FLAG_TOMBSTONE_ENTRY);
}
- slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry );
-
/*
* Sanity check to avoid to delete a non-tombstone or to tombstone again
* a tombstone entry. This should not happen (see bug 561003).
@@ -381,6 +370,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
char *tombstone_dn = compute_entry_tombstone_dn(slapi_entry_get_dn(e->ep_entry),
childuniqueid);
Slapi_Value *tomb_value;
+ Slapi_Entry *e_with_usn = NULL;
nscpEntrySDN = slapi_entry_get_sdn(e->ep_entry);
@@ -422,20 +412,16 @@ ldbm_back_delete( Slapi_PBlock *pb )
slapi_entry_add_value(tombstone->ep_entry, SLAPI_ATTR_OBJECTCLASS, tomb_value);
slapi_value_free(&tomb_value);
- /* retrieve previous entry usn value, if any */
- prev_entryusn_str = slapi_entry_attr_get_charptr(tombstone->ep_entry,
- SLAPI_ATTR_ENTRYUSN_PREV);
- if (prev_entryusn_str) {
- /* discard the previous value from the tombstone entry */
- retval = slapi_entry_delete_string(tombstone->ep_entry,
- SLAPI_ATTR_ENTRYUSN_PREV, prev_entryusn_str);
- if (0 != retval) {
- LDAPDebug( LDAP_DEBUG_TRACE,
- "delete (deleting %s) failed, err=%d\n",
- SLAPI_ATTR_ENTRYUSN, retval, 0) ;
+ /* new usn was set in SLAPI_DELETE_EXISTING_ENTRY by usn code */
+ slapi_pblock_get(pb, SLAPI_DELETE_EXISTING_ENTRY, &e_with_usn);
+ if (e_with_usn) {
+ char *new_usn = slapi_entry_attr_get_charptr(e_with_usn, SLAPI_ATTR_ENTRYUSN);
+ if (new_usn) {
+ slapi_entry_attr_set_charptr(tombstone->ep_entry, SLAPI_ATTR_ENTRYUSN, new_usn);
+ slapi_ch_free_string(&new_usn);
}
+ prev_entryusn_str = slapi_entry_attr_get_charptr(e_with_usn, SLAPI_ATTR_ENTRYUSN_PREV);
}
-
/* XXXggood above used to be: slapi_entry_add_string(tombstone->ep_entry, SLAPI_ATTR_OBJECTCLASS, SLAPI_ATTR_VALUE_TOMBSTONE); */
/* JCMREPL - Add a description of what's going on? */
}
@@ -1032,17 +1018,7 @@ common_return:
* but not if the operation is purging tombstones.
*/
if (!delete_tombstone_entry) {
- if (e) {
- /* set entry in case be-postop plugins need to work on it
- * (e.g., USN) */
- slapi_pblock_get( pb, SLAPI_DELETE_BEPOSTOP_ENTRY, &orig_entry );
- slapi_pblock_set( pb, SLAPI_DELETE_BEPOSTOP_ENTRY, e->ep_entry );
- }
plugin_call_plugins (pb, SLAPI_PLUGIN_BE_POST_DELETE_FN);
- /* set original entry back */
- if (e) {
- slapi_pblock_set( pb, SLAPI_DELETE_BEPOSTOP_ENTRY, orig_entry );
- }
}
/* Need to return to cache after post op plugins are called */
More information about the 389-commits
mailing list