From cebbbf542bc2e52716cb35458682ab083590ccf8 Mon Sep 17 00:00:00 2001 From: Dmitry Yemanov Date: Sun, 12 Feb 2017 17:34:19 +0300 Subject: [PATCH] Rework CORE-5456 / CORE-5457 by reverting back to the v2.5 logic. Previous attempts succeeded only partially. --- src/jrd/RecordSourceNodes.cpp | 16 +++++++++------- src/jrd/RecordSourceNodes.h | 22 +++++++++++++++++++++- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/jrd/RecordSourceNodes.cpp b/src/jrd/RecordSourceNodes.cpp index 640f314f60..c5eb0aca7a 100644 --- a/src/jrd/RecordSourceNodes.cpp +++ b/src/jrd/RecordSourceNodes.cpp @@ -930,7 +930,8 @@ ProcedureSourceNode* ProcedureSourceNode::parse(thread_db* tdbb, CompilerScratch ProcedureSourceNode* node = FB_NEW_POOL(*tdbb->getDefaultPool()) ProcedureSourceNode( *tdbb->getDefaultPool()); - node->procedure = procedure; + node->procedureId = procedure->getId(); + node->isSubRoutine = procedure->isSubRoutine(); node->stream = PAR_context(csb, &node->context); csb->csb_rpt[node->stream].csb_procedure = procedure; @@ -1099,16 +1100,14 @@ ProcedureSourceNode* ProcedureSourceNode::copy(thread_db* tdbb, NodeCopier& copi newSource->targetList = copier.copy(tdbb, targetList); } - jrd_prc* const new_procedure = - MET_lookup_procedure_id(tdbb, procedure->getId(), false, false, 0); - newSource->stream = copier.csb->nextStream(); copier.remap[stream] = newSource->stream; newSource->context = context; - newSource->procedure = new_procedure; + newSource->procedureId = procedureId; + newSource->isSubRoutine = isSubRoutine; newSource->view = view; CompilerScratch::csb_repeat* element = CMP_csb_element(copier.csb, newSource->stream); - element->csb_procedure = new_procedure; + element->csb_procedure = MET_lookup_procedure_id(tdbb, procedureId, false, false, 0); element->csb_view = newSource->view; element->csb_view_stream = copier.remap[0]; @@ -1139,8 +1138,9 @@ void ProcedureSourceNode::pass1Source(thread_db* tdbb, CompilerScratch* csb, Rse pass1(tdbb, csb); - if (!procedure->isSubRoutine()) + if (!isSubRoutine) { + jrd_prc* const procedure = MET_lookup_procedure_id(tdbb, procedureId, false, false, 0); CMP_post_procedure_access(tdbb, csb, procedure); CMP_post_resource(&csb->csb_resources, procedure, Resource::rsc_procedure, procedure->getId()); } @@ -1203,6 +1203,8 @@ ProcedureScan* ProcedureSourceNode::generate(thread_db* tdbb, OptimizerBlk* opt) CompilerScratch::csb_repeat* const csbTail = &csb->csb_rpt[stream]; const string alias = OPT_make_alias(tdbb, csb, csbTail); + jrd_prc* const procedure = MET_lookup_procedure_id(tdbb, procedureId, false, false, 0); + return FB_NEW_POOL(*tdbb->getDefaultPool()) ProcedureScan(csb, alias, stream, procedure, sourceList, targetList, in_msg); } diff --git a/src/jrd/RecordSourceNodes.h b/src/jrd/RecordSourceNodes.h index 30483d3471..f108186975 100644 --- a/src/jrd/RecordSourceNodes.h +++ b/src/jrd/RecordSourceNodes.h @@ -348,7 +348,8 @@ public: sourceList(NULL), targetList(NULL), in_msg(NULL), - procedure(NULL), + procedureId(0), + isSubRoutine(false), view(NULL), context(0) { @@ -410,7 +411,26 @@ public: private: NestConst in_msg; +/* was: jrd_prc* procedure; + + dimitr: Referencing procedures via a pointer is not currently reliable, because + procedures can be removed from the metadata cache after ALTER/DROP. + Usually, this is prevented via the reference counting, but it's incremented + only for compiled requests. Node trees without requests (e.g. computed fields) + are not protected and may end with dead procedure pointers, causing problems + (up to crashing) when they're copied the next time. See CORE-5456 / CORE-5457. + + ExecProcedureNode is a lucky exception because it's never (directly) used in + expressions. + + A better (IMO) solution would be to add a second-level reference counting for + metadata objects since the parsing stage till either request creation or + explicit unload from the metadata cache. But we don't have clearly established + cache management policies yet, so I leave it for the other day. +*/ + USHORT procedureId; + bool isSubRoutine; jrd_rel* view; SSHORT context; };