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@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 */