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
This commit is contained in:
Pierangelo Masarati 2001-07-21 10:53:06 +00:00
parent aec4430d59
commit 9ee9f1e0e1
5 changed files with 43 additions and 20 deletions

View File

@ -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 );
}

View File

@ -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 );
}

View File

@ -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 );
}

View File

@ -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 );
}

View File

@ -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 ));