Rework CORE-5456 / CORE-5457 by reverting back to the v2.5 logic. Pre…

…vious attempts succeeded only partially.

src/jrd/RecordSourceNodes.cpp

@@ -939,7 +939,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;
@@ -1108,16 +1109,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];
 
@@ -1148,8 +1147,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());
 	}
@@ -1212,6 +1212,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);
 }

src/jrd/RecordSourceNodes.h

@@ -354,7 +354,8 @@ class ProcedureSourceNode : public TypedNode<RecordSourceNode, RecordSourceNode:
 		  sourceList(NULL),
 		  targetList(NULL),
 		  in_msg(NULL),
-		  procedure(NULL),
+		  procedureId(0),
+		  isSubRoutine(false),
 		  view(NULL),
 		  context(0)
 	{
@@ -416,7 +417,26 @@ class ProcedureSourceNode : public TypedNode<RecordSourceNode, RecordSourceNode:
 
 private:
 	NestConst<MessageNode> 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;
 };