From: Pierangelo Masarati <ando@openldap.org>
Date: Sat, 21 Jul 2001 10:53:06 +0000 (+0000)
Subject: Reworked again the caching in case of failure.
X-Git-Tag: LDBM_PRE_GIANT_RWLOCK~1224
X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=9ee9f1e0e1332b5ca70cd6c61b694eb278fd4173;p=openldap

Reworked again the caching in case of failure.
Now operations that set the status of an entry to CREATING (add.c, modrdn.c)
need to set it to COMMIT, by calling cache_entry_commit, before returning
the entry itself, otherwise the entry is removed from the cache
and its private data is freed.
Should fix crashes due to add failures as in ITS#1245
---

diff --git a/servers/slapd/back-ldbm/add.c b/servers/slapd/back-ldbm/add.c
index bd9d796952..99dfe0fdb5 100644
--- a/servers/slapd/back-ldbm/add.c
+++ b/servers/slapd/back-ldbm/add.c
@@ -354,7 +354,12 @@ ldbm_back_add(
 
 	send_ldap_result( conn, op, LDAP_SUCCESS,
 		NULL, NULL, NULL, NULL );
+
+	/* marks the entry as committed, so it is added to the cache;
+	 * otherwise it is removed from the cache, but not destroyed;
+	 * it will be destroyed by the caller */
 	rc = 0;
+	cache_entry_commit( e );
 
 return_results:;
 	if (p != NULL) {
@@ -368,10 +373,8 @@ return_results:;
 	}
 
 	if ( rc ) {
-		/* FIXME: remove from cache? */
-		cache_entry_private_destroy_mark( e );
-
-		/* free entry and writer lock */
+		/* in case of error, writer lock is freed 
+		 * and entry's private data is destroyed */
 		cache_return_entry_w( &li->li_cache, e );
 	}
 
diff --git a/servers/slapd/back-ldbm/cache.c b/servers/slapd/back-ldbm/cache.c
index 65c1e4fec2..4227f3be0b 100644
--- a/servers/slapd/back-ldbm/cache.c
+++ b/servers/slapd/back-ldbm/cache.c
@@ -29,9 +29,9 @@ typedef struct ldbm_entry_info {
 	int		lei_state;	/* for the cache */
 #define	CACHE_ENTRY_UNDEFINED	0
 #define CACHE_ENTRY_CREATING	1
-#define CACHE_ENTRY_READY		2
-#define CACHE_ENTRY_DELETED		3
-#define CACHE_ENTRY_DESTROY_PRIVATE	4
+#define CACHE_ENTRY_READY	2
+#define CACHE_ENTRY_DELETED	3
+#define CACHE_ENTRY_COMMITTED	4
 	
 	int		lei_refcnt;	/* # threads ref'ing this entry */
 	Entry	*lei_lrunext;	/* for cache lru list */
@@ -136,17 +136,20 @@ cache_entry_private_init( Entry*e )
 }
 
 /*
- * assumes that the entry is write-locked;marks it i a manner that
- * makes e_private be destroyed at the following cache_return_entry_w,
+ * marks an entry in CREATING state as committed, so it is really returned
+ * to the cache. Otherwise an entry in CREATING state is removed.
+ * Makes e_private be destroyed at the following cache_return_entry_w,
  * but lets the entry untouched (owned by someone else)
  */
 void
-cache_entry_private_destroy_mark( Entry *e )
+cache_entry_commit( Entry *e )
 {
 	assert( e );
 	assert( e->e_private );
+	assert( LEI(e)->lei_state == CACHE_ENTRY_CREATING );
+	/* assert( LEI(e)->lei_refcnt == 1 ); */
 
-	LEI(e)->lei_state = CACHE_ENTRY_DESTROY_PRIVATE;
+	LEI(e)->lei_state = CACHE_ENTRY_COMMITTED;
 }
 
 static int
@@ -165,7 +168,7 @@ void
 cache_return_entry_rw( Cache *cache, Entry *e, int rw )
 {
 	ID id;
-	int refcnt;
+	int refcnt, freeit = 1;
 
 	/* set cache mutex */
 	ldap_pvt_thread_mutex_lock( &cache->c_mutex );
@@ -177,7 +180,18 @@ cache_return_entry_rw( Cache *cache, Entry *e, int rw )
 	id = e->e_id;
 	refcnt = --LEI(e)->lei_refcnt;
 
-	if ( LEI(e)->lei_state == CACHE_ENTRY_CREATING ) {
+	/*
+	 * if the entry is returned when in CREATING state, it is deleted
+	 * but not freed because it may belong to someone else (do_add,
+	 * for instance)
+	 */
+	if (  LEI(e)->lei_state == CACHE_ENTRY_CREATING ) {
+		cache_delete_entry_internal( cache, e );
+		freeit = 0;
+		/* now the entry is in DELETED state */
+	}
+
+	if ( LEI(e)->lei_state == CACHE_ENTRY_COMMITTED ) {
 		LEI(e)->lei_state = CACHE_ENTRY_READY;
 
 		/* free cache mutex */
@@ -194,8 +208,7 @@ cache_return_entry_rw( Cache *cache, Entry *e, int rw )
 #endif
 
 
-	} else if ( LEI(e)->lei_state == CACHE_ENTRY_DELETED
-		 || LEI(e)->lei_state == CACHE_ENTRY_DESTROY_PRIVATE ) {
+	} else if ( LEI(e)->lei_state == CACHE_ENTRY_DELETED ) {
 		if( refcnt > 0 ) {
 			/* free cache mutex */
 			ldap_pvt_thread_mutex_unlock( &cache->c_mutex );
@@ -212,10 +225,8 @@ cache_return_entry_rw( Cache *cache, Entry *e, int rw )
 
 
 		} else {
-			int state = LEI(e)->lei_state;
-
 			cache_entry_private_destroy( e );
-			if ( state == CACHE_ENTRY_DELETED ) {
+			if ( freeit ) {
 				entry_free( e );
 			}
 
diff --git a/servers/slapd/back-ldbm/id2entry.c b/servers/slapd/back-ldbm/id2entry.c
index a0e11c8073..69a5ceb3da 100644
--- a/servers/slapd/back-ldbm/id2entry.c
+++ b/servers/slapd/back-ldbm/id2entry.c
@@ -274,6 +274,9 @@ id2entry_rw( Backend *be, ID id, int rw )
 		rw ? "w" : "r", id, (unsigned long) e );
 #endif
 
+	/* marks the entry as committed, so it will get added to the cache
+	 * when the lock is released */
+	cache_entry_commit( e );
 
 	return( e );
 }
diff --git a/servers/slapd/back-ldbm/modrdn.c b/servers/slapd/back-ldbm/modrdn.c
index f10a4b353d..78b2ef0962 100644
--- a/servers/slapd/back-ldbm/modrdn.c
+++ b/servers/slapd/back-ldbm/modrdn.c
@@ -847,6 +847,7 @@ ldbm_back_modrdn(
 	    
 	    goto return_results;
 	}
+	rc = -1;
 	
 	(void) cache_update_entry( &li->li_cache, e );
 
@@ -856,7 +857,6 @@ ldbm_back_modrdn(
 
 	/* id2entry index */
 	if ( id2entry_add( be, e ) != 0 ) {
-		entry_free( e );
 		send_ldap_result( conn, op, LDAP_OTHER,
 			NULL, "entry update failed", NULL, NULL );
 		goto return_results;
@@ -865,6 +865,7 @@ ldbm_back_modrdn(
 	send_ldap_result( conn, op, LDAP_SUCCESS,
 		NULL, NULL, NULL, NULL );
 	rc = 0;
+	cache_entry_commit( e );
 
 return_results:
 	if( new_dn != NULL ) free( new_dn );
@@ -917,5 +918,10 @@ return_results:
 
 	/* free entry and writer lock */
 	cache_return_entry_w( &li->li_cache, e );
+	if ( rc ) {
+		/* if rc != 0 the entry is uncached and its private data 
+		 * is destroyed; the entry must be freed */
+		entry_free( e );
+	}
 	return( rc );
 }
diff --git a/servers/slapd/back-ldbm/proto-back-ldbm.h b/servers/slapd/back-ldbm/proto-back-ldbm.h
index 7d798fe55a..1f9426f2a5 100644
--- a/servers/slapd/back-ldbm/proto-back-ldbm.h
+++ b/servers/slapd/back-ldbm/proto-back-ldbm.h
@@ -51,7 +51,7 @@ int cache_update_entry LDAP_P(( Cache *cache, Entry *e ));
 void cache_return_entry_rw LDAP_P(( Cache *cache, Entry *e, int rw ));
 #define cache_return_entry_r(c, e) cache_return_entry_rw((c), (e), 0)
 #define cache_return_entry_w(c, e) cache_return_entry_rw((c), (e), 1)
-void cache_entry_private_destroy_mark LDAP_P(( Entry *e ));
+void cache_entry_commit LDAP_P(( Entry *e ));
 
 ID cache_find_entry_dn2id LDAP_P(( Backend *be, Cache *cache, const char *dn ));
 ID cache_find_entry_ndn2id LDAP_P(( Backend *be, Cache *cache, const char *ndn ));