Skip to content

Commit a30cb95

Browse files
committed
Reworked PR #7426. This fixes the crash during second invocation of external trigger. Using a temporary vector is a bad idea because it owns the trigger object but external triggers store a back pointer to Jrd::Trigger, thus implying it being persistent.
1 parent 32f7dfa commit a30cb95

File tree

2 files changed

+25
-49
lines changed

2 files changed

+25
-49
lines changed

src/jrd/exe.cpp

Lines changed: 24 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -590,58 +590,19 @@ void EXE_execute_db_triggers(thread_db* tdbb, jrd_tra* transaction, TriggerActio
590590
// Execute DDL triggers.
591591
void EXE_execute_ddl_triggers(thread_db* tdbb, jrd_tra* transaction, bool preTriggers, int action)
592592
{
593-
Jrd::Attachment* attachment = tdbb->getAttachment();
593+
const auto attachment = tdbb->getAttachment();
594594

595-
// Our caller verifies (ATT_no_db_triggers) if DDL triggers should not run.
595+
// Our caller verifies (ATT_no_db_triggers) if DDL triggers should not run
596596

597597
if (attachment->att_ddl_triggers)
598598
{
599-
TrigVector triggers;
600-
TrigVector* triggersPtr = &triggers;
601-
HalfStaticArray<Trigger*, 4> cachedTriggers;
602-
603-
for (auto& trigger : *attachment->att_ddl_triggers)
604-
{
605-
const auto type = trigger.type & ~TRIGGER_TYPE_MASK;
606-
const bool preTrigger = ((type & 1) == 0);
607-
608-
if ((type & (1LL << action)) && (preTriggers == preTrigger))
609-
{
610-
triggers.add() = trigger;
611-
cachedTriggers.add(&trigger);
612-
}
613-
}
614-
615-
if (triggers.hasData())
616-
{
617-
FbLocalStatus tempStatus;
618-
619-
jrd_tra* const oldTransaction = tdbb->getTransaction();
620-
tdbb->setTransaction(transaction);
621-
622-
try
623-
{
624-
EXE_execute_triggers(tdbb, &triggersPtr, NULL, NULL, TRIGGER_DDL,
625-
preTriggers ? StmtNode::PRE_TRIG : StmtNode::POST_TRIG);
626-
}
627-
catch (const Exception& ex)
628-
{
629-
ex.stuffException(&tempStatus);
630-
}
631-
632-
tdbb->setTransaction(oldTransaction);
633-
634-
// Triggers could be compiled inside EXE_execute_triggers(),
635-
// so ensure the new pointers are copied back to the cache
636-
fb_assert(triggers.getCount() == cachedTriggers.getCount());
637-
for (unsigned i = 0; i < triggers.getCount(); i++)
638-
{
639-
*cachedTriggers[i] = triggers[i];
640-
triggers[i].extTrigger = nullptr; // avoid deletion inside d'tor
641-
}
599+
AutoSetRestore2<jrd_tra*, thread_db> tempTrans(tdbb,
600+
&thread_db::getTransaction,
601+
&thread_db::setTransaction,
602+
transaction);
642603

643-
tempStatus.check();
644-
}
604+
EXE_execute_triggers(tdbb, &attachment->att_ddl_triggers, NULL, NULL, TRIGGER_DDL,
605+
preTriggers ? StmtNode::PRE_TRIG : StmtNode::POST_TRIG, action);
645606
}
646607
}
647608

@@ -1166,7 +1127,8 @@ void EXE_execute_triggers(thread_db* tdbb,
11661127
record_param* old_rpb,
11671128
record_param* new_rpb,
11681129
TriggerAction trigger_action,
1169-
StmtNode::WhichTrigger which_trig)
1130+
StmtNode::WhichTrigger which_trig,
1131+
int ddl_action)
11701132
{
11711133
/**************************************
11721134
*
@@ -1219,6 +1181,20 @@ void EXE_execute_triggers(thread_db* tdbb,
12191181
{
12201182
for (TrigVector::iterator ptr = vector->begin(); ptr != vector->end(); ++ptr)
12211183
{
1184+
if (trigger_action == TRIGGER_DDL && ddl_action)
1185+
{
1186+
// Skip triggers not matching our action
1187+
1188+
fb_assert(which_trig == StmtNode::PRE_TRIG || which_trig == StmtNode::POST_TRIG);
1189+
const bool preTriggers = (which_trig == StmtNode::PRE_TRIG);
1190+
1191+
const auto type = ptr->type & ~TRIGGER_TYPE_MASK;
1192+
const bool preTrigger = ((type & 1) == 0);
1193+
1194+
if (!(type & (1LL << ddl_action)) || preTriggers != preTrigger)
1195+
continue;
1196+
}
1197+
12221198
ptr->compile(tdbb);
12231199

12241200
trigger = ptr->statement->findRequest(tdbb);

src/jrd/exe_proto.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ const Jrd::StmtNode* EXE_looper(Jrd::thread_db* tdbb, Jrd::Request* request,
4646
const Jrd::StmtNode* in_node);
4747

4848
void EXE_execute_triggers(Jrd::thread_db*, Jrd::TrigVector**, Jrd::record_param*, Jrd::record_param*,
49-
enum TriggerAction, Jrd::StmtNode::WhichTrigger);
49+
enum TriggerAction, Jrd::StmtNode::WhichTrigger, int = 0);
5050

5151
void EXE_receive(Jrd::thread_db*, Jrd::Request*, USHORT, ULONG, void*, bool = false);
5252
void EXE_release(Jrd::thread_db*, Jrd::Request*);

0 commit comments

Comments
 (0)