From 9ee9f1e0e1332b5ca70cd6c61b694eb278fd4173 Mon Sep 17 00:00:00 2001 From: Pierangelo Masarati Date: Sat, 21 Jul 2001 10:53:06 +0000 Subject: [PATCH] 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 --- servers/slapd/back-ldbm/add.c | 11 ++++--- servers/slapd/back-ldbm/cache.c | 39 +++++++++++++++-------- servers/slapd/back-ldbm/id2entry.c | 3 ++ servers/slapd/back-ldbm/modrdn.c | 8 ++++- servers/slapd/back-ldbm/proto-back-ldbm.h | 2 +- 5 files changed, 43 insertions(+), 20 deletions(-) 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 )); -- 2.39.5