From ebdcd20d0683082c87748753c16bf1b77182538a Mon Sep 17 00:00:00 2001 From: skidder Date: Fri, 4 Oct 2002 22:08:43 +0000 Subject: [PATCH] Fixed resource leaks in DDL recursive procedure handling which caused some DDL to fail --- src/jrd/dfw.epp | 14 +++++- src/jrd/jrd.h | 7 ++- src/jrd/met.epp | 111 +++++++++++++++++++++++++++++++++----------- src/jrd/met_proto.h | 2 +- 4 files changed, 103 insertions(+), 31 deletions(-) diff --git a/src/jrd/dfw.epp b/src/jrd/dfw.epp index 525d070d52..f916d990bb 100644 --- a/src/jrd/dfw.epp +++ b/src/jrd/dfw.epp @@ -2292,7 +2292,7 @@ static bool delete_index(TDBB tdbb, SSHORT phase, DFW work, TRA transaction) wait = (transaction->tra_flags & TRA_nowait) ? FALSE : TRUE; // Try to clear trigger cache to release lock if (index->idl_count) - MET_clear_cache(tdbb); + MET_clear_cache(tdbb, NULL); if (index->idl_count || !LCK_lock_non_blocking(tdbb, index->idl_lock, LCK_EX, wait)) { @@ -2542,8 +2542,15 @@ static bool delete_procedure( TDBB tdbb, return false; } + if (procedure->prc_use_count) + MET_clear_cache(tdbb, procedure); + + // "OBJECT IN USE" error should be emitted in this case, but some more + // code needs to be reviewed for that. Now you can drop procedure + // used by other compiled requests if (procedure->prc_use_count) { + gds__log("Deleting unfreed procedure %s",work->dfw_name); MET_delete_dependencies(tdbb, work->dfw_name, obj_procedure); LCK_release(tdbb, procedure->prc_existence_lock); @@ -2678,6 +2685,8 @@ static bool delete_relation(TDBB tdbb, SSHORT phase, DFW work, TRA transaction) } wait = (transaction->tra_flags & TRA_nowait) ? FALSE : TRUE; + if (relation->rel_use_count) + MET_clear_cache(tdbb, NULL); if (relation->rel_use_count || (relation->rel_existence_lock && !LCK_convert_non_blocking( tdbb, @@ -3879,8 +3888,11 @@ static bool modify_procedure( TDBB tdbb, } THREAD_ENTER; #endif /* SUPERSERVER */ + if (procedure->prc_use_count) + MET_clear_cache(tdbb,procedure); if (procedure->prc_use_count) { + gds__log("Modifying unfreed procedure %s",work->dfw_name); USHORT prc_alter_count = procedure->prc_alter_count; if (prc_alter_count > MAX_PROC_ALTER) { diff --git a/src/jrd/jrd.h b/src/jrd/jrd.h index c65174a31b..6ad106ef95 100644 --- a/src/jrd/jrd.h +++ b/src/jrd/jrd.h @@ -492,7 +492,12 @@ class prc : public pool_alloc_rpt vec* prc_output_fields; /* vector of field blocks */ struct req *prc_request; /* compiled procedure request */ class str *prc_security_name; /* pointer to security class name for procedure */ - USHORT prc_use_count; /* requests compiled with relation */ + USHORT prc_use_count; /* requests compiled with procedure */ + SSHORT prc_int_use_count; /* number of procedures compiled with procedure, set and + used internally in the MET_clear_cache procedure + no code should rely on value of this field + (it will usually be 0) + */ struct lck *prc_existence_lock; /* existence lock, if any */ class str *prc_name; /* pointer to ascic name */ USHORT prc_alter_count; /* No. of times the procedure was altered */ diff --git a/src/jrd/met.epp b/src/jrd/met.epp index 482b94c022..d7a5afacd7 100644 --- a/src/jrd/met.epp +++ b/src/jrd/met.epp @@ -35,7 +35,7 @@ * 2002-09-16 Nickolay Samofatov - Deferred trigger compilation changes */ /* -$Id: met.epp,v 1.24 2002-09-30 19:19:30 skidder Exp $ +$Id: met.epp,v 1.25 2002-10-04 22:08:43 skidder Exp $ */ // This MUST be at the top of the file #ifdef DARWIN @@ -74,7 +74,6 @@ $Id: met.epp,v 1.24 2002-09-30 19:19:30 skidder Exp $ #include "../jrd/lls.h" #include "../jrd/intl.h" #include "../jrd/align.h" -#include "../jrd/jrn.h" #include "../jrd/flu.h" #include "../jrd/blob_filter.h" #include "../intl/charsets.h" @@ -221,16 +220,38 @@ void MET_update_partners(TDBB tdbb) } } -void MET_clear_cache(TDBB tdbb) +void adjust_dependencies(PRC procedure) { + if (procedure->prc_int_use_count==-1) + // Already processed + return; + procedure->prc_int_use_count=-1; // Mark as undeletable + for (RSC resource = procedure->prc_request->req_resources; resource; + resource = resource->rsc_next) + { + if (resource->rsc_type == rsc_procedure) { + procedure = resource->rsc_prc; + if (procedure->prc_int_use_count==procedure->prc_use_count) { + // Mark it and all dependent procedures as undeletable + adjust_dependencies(procedure); + } + else + procedure->prc_int_use_count=-1; + } + } +} + +void MET_clear_cache(TDBB tdbb, PRC proc) { /************************************** * - * M E T _ c l e a r _ t r i g g e r _ c a c h e + * M E T _ c l e a r _ c a c h e * ************************************** * * Functional description * Try to release all resources locked by cached triggers + * do not remove proc procedure from cache because it will be + * handled by the caller * **************************************/ DBB dbb; @@ -242,7 +263,7 @@ void MET_clear_cache(TDBB tdbb) dbb = tdbb->tdbb_database; relations = dbb->dbb_relations; - + for (ptr = relations->begin(), end = relations->end(); ptr < end; ptr++) { relation = REL(*ptr); @@ -258,27 +279,63 @@ void MET_clear_cache(TDBB tdbb) VEC procedures; PRC procedure; - if ( (procedures = dbb->dbb_procedures) ) - do { - procedure = NULL; - for (ptr = procedures->begin(), end = procedures->end(); - ptr < end; ptr++) + if ( (procedures = dbb->dbb_procedures) ) { + /* Walk procedures and calculate internal dependencies */ + for (ptr = procedures->begin(), end = procedures->end(); + ptr < end; ptr++) + { + if ( (procedure = PRC(*ptr)) && !(procedure->prc_flags & PRC_obsolete)) { - if ( (procedure = PRC(*ptr)) && (procedure->prc_use_count == 0) && - !(procedure->prc_flags & PRC_obsolete)) + for (RSC resource = procedure->prc_request->req_resources; resource; + resource = resource->rsc_next) { - CMP_release(tdbb, procedure->prc_request); - procedure->prc_flags &= ~PRC_being_altered; - MET_remove_procedure(tdbb, procedure->prc_id, procedure); - // Begin iteration again because vector contents was modified - // There are ways to do it more efficiently, but such condition - // currently happens very rarely. Only when procedure is created - // and we are trying to drop one of indices it uses w/o server - // restart - break; + if (resource->rsc_type == rsc_procedure) + resource->rsc_prc->prc_int_use_count++; } } - } while (procedure); + } + + /* Walk procedures again and adjust dependencies for procedures which will not be removed */ + for (ptr = procedures->begin(), end = procedures->end(); + ptr < end; ptr++) + { + if ( (procedure = PRC(*ptr)) && !(procedure->prc_flags & PRC_obsolete) && + procedure->prc_use_count != procedure->prc_int_use_count ) + { + adjust_dependencies(procedure); + } + } + + /* Deallocate all used requests */ + Firebird::vector temp(0); + for (ptr = procedures->begin(), end = procedures->end(); + ptr < end; ptr++) + { + if ( (procedure = PRC(*ptr)) && + !(procedure->prc_flags & PRC_obsolete)) + { + + if ( procedure->prc_use_count == procedure->prc_int_use_count ) + { + CMP_release(tdbb, procedure->prc_request); + procedure->prc_request = NULL; + temp.insert(temp.end(),procedure); + } else + // Leave it in state 0 to avoid extra pass next time to clear it + procedure->prc_int_use_count = 0; + } + } + + /* Remove all affected procedures from the cache */ + Firebird::vector::iterator cur, fin; + for (cur = temp.begin(), fin = temp.end(); cur < fin; cur++) { + procedure = *cur; + if (procedure==proc) + continue; + procedure->prc_flags &= ~PRC_being_altered; // Just a safety sake + MET_remove_procedure(tdbb, procedure->prc_id, procedure); + } + } } @@ -2597,6 +2654,7 @@ PRC MET_procedure(TDBB tdbb, int id, USHORT flags) } procedure->prc_id = P.RDB$PROCEDURE_ID; procedure->prc_use_count = 0; + procedure->prc_int_use_count = 0; if (!P.RDB$SECURITY_CLASS.NULL) { procedure->prc_security_name = @@ -2935,8 +2993,9 @@ void MET_release_existence( REL relation) * existence lock and mark deleted. * **************************************/ + if (relation->rel_use_count) relation->rel_use_count--; - if (!relation->rel_use_count || !--relation->rel_use_count) + if (!relation->rel_use_count) if (relation->rel_flags & REL_blocking) LCK_re_post(relation->rel_existence_lock); } @@ -2980,11 +3039,7 @@ void MET_remove_procedure( TDBB tdbb, int id, PRC procedure) /* Procedure that is being altered may have references to it by other procedures via pointer to current meta data structure, so don't loose the structure or the pointer. */ -// Nickolay Samofatov, 28 Sep 2002. Enabled this code. Why was it disabled ? -// Its absence caused coredumps in some cases because its request and other -// data is freed after this call and procedure block is no longer valid - - if (!(procedure->prc_flags & PRC_being_altered)) + if ((procedure == (PRC) (*vector)[id]) && !(procedure->prc_flags & PRC_being_altered)) (*vector)[id] = (BLK) NULL_PTR; /* deallocate all structure which were allocated. The procedure diff --git a/src/jrd/met_proto.h b/src/jrd/met_proto.h index 940589c062..bb69dff96d 100644 --- a/src/jrd/met_proto.h +++ b/src/jrd/met_proto.h @@ -76,7 +76,7 @@ extern BOOLEAN MET_relation_owns_trigger (TDBB, const TEXT *, const TEXT *); extern BOOLEAN MET_relation_default_class (TDBB, const TEXT *, const TEXT *); void MET_release_existence(struct rel *); void MET_release_triggers(TDBB, TRIG_VEC *); -void MET_clear_cache(TDBB); +void MET_clear_cache(TDBB, PRC); void MET_remove_procedure(TDBB, int, PRC); void MET_revoke(TDBB, struct tra *, TEXT *, TEXT *, TEXT *); TEXT*MET_save_name(TDBB, CONST TEXT*);