Skip to content

Commit ffb9f22

Browse files
dyemanovdyemanov
committed
This should fix CORE-6382: Triggers accessing a table prevent concurrent DDL command from dropping that table
1 parent b283d80 commit ffb9f22

File tree

9 files changed

+130
-156
lines changed

9 files changed

+130
-156
lines changed

‎src/dsql/StmtNodes.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2481,7 +2481,7 @@ void EraseNode::pass1Erase(thread_db* tdbb, CompilerScratch* csb, EraseNode* nod
24812481
if (parent)
24822482
priv |= SCL_select;
24832483

2484-
RefPtr<const TrigVector> trigger(relation->rel_pre_erase ?
2484+
RefPtr<TrigVector> trigger(relation->rel_pre_erase ?
24852485
relation->rel_pre_erase : relation->rel_post_erase);
24862486

24872487
// If we have a view with triggers, let's expand it.
@@ -6426,7 +6426,7 @@ void ModifyNode::pass1Modify(thread_db* tdbb, CompilerScratch* csb, ModifyNode*
64266426
if (parent)
64276427
priv |= SCL_select;
64286428

6429-
RefPtr<const TrigVector> trigger(relation->rel_pre_modify ?
6429+
RefPtr<TrigVector> trigger(relation->rel_pre_modify ?
64306430
relation->rel_pre_modify : relation->rel_post_modify);
64316431

64326432
// If we have a view with triggers, let's expand it.
@@ -7289,7 +7289,7 @@ bool StoreNode::pass1Store(thread_db* tdbb, CompilerScratch* csb, StoreNode* nod
72897289

72907290
postTriggerAccess(csb, relation, ExternalAccess::exa_insert, view);
72917291

7292-
RefPtr<const TrigVector> trigger(relation->rel_pre_store ?
7292+
RefPtr<TrigVector> trigger(relation->rel_pre_store ?
72937293
relation->rel_pre_store : relation->rel_post_store);
72947294

72957295
// Check out insert. If this is an insert thru a view, verify the view by checking for read

‎src/jrd/Relation.cpp

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "firebird.h"
2424
#include "../jrd/Relation.h"
2525

26+
#include "../jrd/met.h"
2627
#include "../jrd/tra.h"
2728
#include "../jrd/btr_proto.h"
2829
#include "../jrd/dpm_proto.h"
@@ -325,6 +326,45 @@ bool jrd_rel::hasTriggers() const
325326
return false;
326327
}
327328

329+
void jrd_rel::releaseTriggers(thread_db* tdbb, bool destroy)
330+
{
331+
MET_release_triggers(tdbb, &rel_pre_store, destroy);
332+
MET_release_triggers(tdbb, &rel_post_store, destroy);
333+
MET_release_triggers(tdbb, &rel_pre_erase, destroy);
334+
MET_release_triggers(tdbb, &rel_post_erase, destroy);
335+
MET_release_triggers(tdbb, &rel_pre_modify, destroy);
336+
MET_release_triggers(tdbb, &rel_post_modify, destroy);
337+
}
338+
339+
void jrd_rel::replaceTriggers(thread_db* tdbb, TrigVector** triggers)
340+
{
341+
TrigVector* tmp_vector;
342+
343+
tmp_vector = rel_pre_store;
344+
rel_pre_store = triggers[TRIGGER_PRE_STORE];
345+
MET_release_triggers(tdbb, &tmp_vector, true);
346+
347+
tmp_vector = rel_post_store;
348+
rel_post_store = triggers[TRIGGER_POST_STORE];
349+
MET_release_triggers(tdbb, &tmp_vector, true);
350+
351+
tmp_vector = rel_pre_erase;
352+
rel_pre_erase = triggers[TRIGGER_PRE_ERASE];
353+
MET_release_triggers(tdbb, &tmp_vector, true);
354+
355+
tmp_vector = rel_post_erase;
356+
rel_post_erase = triggers[TRIGGER_POST_ERASE];
357+
MET_release_triggers(tdbb, &tmp_vector, true);
358+
359+
tmp_vector = rel_pre_modify;
360+
rel_pre_modify = triggers[TRIGGER_PRE_MODIFY];
361+
MET_release_triggers(tdbb, &tmp_vector, true);
362+
363+
tmp_vector = rel_post_modify;
364+
rel_post_modify = triggers[TRIGGER_POST_MODIFY];
365+
MET_release_triggers(tdbb, &tmp_vector, true);
366+
}
367+
328368
Lock* jrd_rel::createLock(thread_db* tdbb, MemoryPool* pool, jrd_rel* relation, lck_t lckType, bool noAst)
329369
{
330370
if (!pool)

‎src/jrd/Relation.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,8 @@ class jrd_rel : public pool_alloc<type_rel>
331331
explicit jrd_rel(MemoryPool& p);
332332

333333
bool hasTriggers() const;
334+
void releaseTriggers(thread_db* tdbb, bool destroy);
335+
void replaceTriggers(thread_db* tdbb, TrigVector** triggers);
334336

335337
static Lock* createLock(thread_db* tdbb, MemoryPool* pool, jrd_rel* relation, lck_t, bool);
336338
static int blocking_ast_gcLock(void*);

‎src/jrd/dfw.epp

Lines changed: 10 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,6 @@ static bool formatsAreEqual(const Format*, const Format*);
500500
static bool find_depend_in_dfw(thread_db*, TEXT*, USHORT, USHORT, jrd_tra*);
501501
static void get_array_desc(thread_db*, const TEXT*, Ods::InternalArrayDesc*);
502502
static void get_trigger_dependencies(DeferredWork*, bool, jrd_tra*);
503-
static void load_trigs(thread_db*, jrd_rel*, TrigVector**);
504503
static Format* make_format(thread_db*, jrd_rel*, USHORT *, TemporaryField*);
505504
static void put_summary_blob(thread_db* tdbb, blb*, enum rsr_t, bid*, jrd_tra*);
506505
static void put_summary_record(thread_db* tdbb, blb*, enum rsr_t, const UCHAR*, USHORT);
@@ -3839,12 +3838,12 @@ static bool create_trigger(thread_db* tdbb, SSHORT phase, DeferredWork* work, jr
38393838
if ((arg->dfw_id & TRIGGER_TYPE_MASK) == TRIGGER_TYPE_DB)
38403839
{
38413840
unsigned triggerKind = arg->dfw_id & ~TRIGGER_TYPE_DB;
3842-
MET_release_triggers(tdbb, &tdbb->getAttachment()->att_triggers[triggerKind]);
3841+
MET_release_triggers(tdbb, &tdbb->getAttachment()->att_triggers[triggerKind], true);
38433842
MET_load_db_triggers(tdbb, triggerKind);
38443843
}
38453844
else if ((arg->dfw_id & TRIGGER_TYPE_MASK) == TRIGGER_TYPE_DDL)
38463845
{
3847-
MET_release_triggers(tdbb, &tdbb->getAttachment()->att_ddl_triggers);
3846+
MET_release_triggers(tdbb, &tdbb->getAttachment()->att_ddl_triggers, true);
38483847
MET_load_ddl_triggers(tdbb);
38493848
}
38503849
}
@@ -4964,12 +4963,7 @@ static bool delete_relation(thread_db* tdbb, SSHORT phase, DeferredWork* work, j
49644963
}
49654964

49664965
// Release relation triggers
4967-
MET_release_triggers(tdbb, &relation->rel_pre_store);
4968-
MET_release_triggers(tdbb, &relation->rel_post_store);
4969-
MET_release_triggers(tdbb, &relation->rel_pre_erase);
4970-
MET_release_triggers(tdbb, &relation->rel_post_erase);
4971-
MET_release_triggers(tdbb, &relation->rel_pre_modify);
4972-
MET_release_triggers(tdbb, &relation->rel_post_modify);
4966+
relation->releaseTriggers(tdbb, true);
49734967

49744968
break;
49754969
}
@@ -5452,48 +5446,6 @@ static void get_trigger_dependencies(DeferredWork* work, bool compile, jrd_tra*
54525446
}
54535447

54545448

5455-
static void load_trigs(thread_db* tdbb, jrd_rel* relation, TrigVector** triggers)
5456-
{
5457-
/**************************************
5458-
*
5459-
* l o a d _ t r i g s
5460-
*
5461-
**************************************
5462-
*
5463-
* Functional description
5464-
* We have just loaded the triggers onto the local vector
5465-
* triggers. Its now time to place them at their rightful
5466-
* place ie the relation block.
5467-
*
5468-
**************************************/
5469-
TrigVector* tmp_vector;
5470-
5471-
tmp_vector = relation->rel_pre_store;
5472-
relation->rel_pre_store = triggers[TRIGGER_PRE_STORE];
5473-
MET_release_triggers(tdbb, &tmp_vector);
5474-
5475-
tmp_vector = relation->rel_post_store;
5476-
relation->rel_post_store = triggers[TRIGGER_POST_STORE];
5477-
MET_release_triggers(tdbb, &tmp_vector);
5478-
5479-
tmp_vector = relation->rel_pre_erase;
5480-
relation->rel_pre_erase = triggers[TRIGGER_PRE_ERASE];
5481-
MET_release_triggers(tdbb, &tmp_vector);
5482-
5483-
tmp_vector = relation->rel_post_erase;
5484-
relation->rel_post_erase = triggers[TRIGGER_POST_ERASE];
5485-
MET_release_triggers(tdbb, &tmp_vector);
5486-
5487-
tmp_vector = relation->rel_pre_modify;
5488-
relation->rel_pre_modify = triggers[TRIGGER_PRE_MODIFY];
5489-
MET_release_triggers(tdbb, &tmp_vector);
5490-
5491-
tmp_vector = relation->rel_post_modify;
5492-
relation->rel_post_modify = triggers[TRIGGER_POST_MODIFY];
5493-
MET_release_triggers(tdbb, &tmp_vector);
5494-
}
5495-
5496-
54975449
static Format* make_format(thread_db* tdbb, jrd_rel* relation, USHORT* version, TemporaryField* stack)
54985450
{
54995451
/**************************************
@@ -6075,8 +6027,11 @@ static bool make_version(thread_db* tdbb, SSHORT phase, DeferredWork* work, jrd_
60756027
if (!relation)
60766028
return false;
60776029

6030+
// We have just loaded the triggers onto the local vector triggers.
6031+
// It's now time to place them at their rightful place inside the relation block.
6032+
60786033
if (!(relation->rel_flags & REL_sys_trigs_being_loaded))
6079-
load_trigs(tdbb, relation, triggers);
6034+
relation->replaceTriggers(tdbb, triggers);
60806035

60816036
// in case somebody changed the view definition or a computed
60826037
// field, reset the dependencies by deleting the current ones
@@ -6170,12 +6125,12 @@ static bool modify_trigger(thread_db* tdbb, SSHORT phase, DeferredWork* work, jr
61706125
if (arg && (arg->dfw_id & TRIGGER_TYPE_MASK) == TRIGGER_TYPE_DB)
61716126
{
61726127
unsigned triggerKind = arg->dfw_id & ~TRIGGER_TYPE_DB;
6173-
MET_release_triggers(tdbb, &tdbb->getAttachment()->att_triggers[triggerKind]);
6128+
MET_release_triggers(tdbb, &tdbb->getAttachment()->att_triggers[triggerKind], true);
61746129
MET_load_db_triggers(tdbb, triggerKind);
61756130
}
61766131
else if ((arg->dfw_id & TRIGGER_TYPE_MASK) == TRIGGER_TYPE_DDL)
61776132
{
6178-
MET_release_triggers(tdbb, &tdbb->getAttachment()->att_ddl_triggers);
6133+
MET_release_triggers(tdbb, &tdbb->getAttachment()->att_ddl_triggers, true);
61796134
MET_load_ddl_triggers(tdbb);
61806135
}
61816136
}
@@ -6217,7 +6172,7 @@ static bool modify_trigger(thread_db* tdbb, SSHORT phase, DeferredWork* work, jr
62176172
for (FB_SIZE_T j = 0; j < triggers[i]->getCount(); ++j)
62186173
(*triggers[i])[j].compile(tdbb);
62196174

6220-
MET_release_triggers(tdbb, &triggers[i]);
6175+
MET_release_triggers(tdbb, &triggers[i], true);
62216176
}
62226177
}
62236178

‎src/jrd/exe.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1184,12 +1184,12 @@ void EXE_execute_triggers(thread_db* tdbb,
11841184
}
11851185

11861186
if (vector != *triggers)
1187-
MET_release_triggers(tdbb, &vector);
1187+
MET_release_triggers(tdbb, &vector, true);
11881188
}
11891189
catch (const Firebird::Exception& ex)
11901190
{
11911191
if (vector != *triggers)
1192-
MET_release_triggers(tdbb, &vector);
1192+
MET_release_triggers(tdbb, &vector, true);
11931193

11941194
if (trigger)
11951195
{

‎src/jrd/jrd.cpp

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -854,6 +854,11 @@ namespace
854854
#define TEXT SCHAR
855855
#endif // WIN_NT
856856

857+
bool Trigger::isActive() const
858+
{
859+
return statement && statement->isActive();
860+
}
861+
857862
void Trigger::compile(thread_db* tdbb)
858863
{
859864
SET_TDBB(tdbb);
@@ -941,7 +946,7 @@ void Trigger::release(thread_db* tdbb)
941946
extTrigger = NULL;
942947
}
943948

944-
if (blr.getCount() == 0 || !statement || statement->isActive())
949+
if (blr.isEmpty() || !statement || statement->isActive())
945950
return;
946951

947952
statement->release(tdbb);
@@ -9367,25 +9372,36 @@ void JRD_cancel_operation(thread_db* /*tdbb*/, Jrd::Attachment* attachment, int
93679372
}
93689373

93699374

9370-
void TrigVector::release() const
9375+
bool TrigVector::hasActive() const
9376+
{
9377+
for (const_iterator iter = begin(); iter != end(); ++iter)
9378+
{
9379+
if (iter->isActive())
9380+
return true;
9381+
}
9382+
9383+
return false;
9384+
}
9385+
9386+
9387+
void TrigVector::decompile(thread_db* tdbb)
9388+
{
9389+
for (iterator iter = begin(); iter != end(); ++iter)
9390+
iter->release(tdbb);
9391+
}
9392+
9393+
9394+
void TrigVector::release()
93719395
{
93729396
release(JRD_get_thread_data());
93739397
}
93749398

93759399

9376-
void TrigVector::release(thread_db* tdbb) const
9400+
void TrigVector::release(thread_db* tdbb)
93779401
{
93789402
if (--useCount == 0)
93799403
{
9380-
const const_iterator e = end();
9381-
9382-
for (const_iterator t = begin(); t != e; ++t)
9383-
{
9384-
JrdStatement* stmt = t->statement;
9385-
if (stmt)
9386-
stmt->release(tdbb);
9387-
}
9388-
9404+
decompile(tdbb);
93899405
delete this;
93909406
}
93919407
}

‎src/jrd/jrd.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,8 @@ class Trigger
156156
Nullable<bool> ssDefiner;
157157
MetaName owner; // Owner for SQL SECURITY
158158

159+
bool isActive() const;
160+
159161
void compile(thread_db*); // Ensure that trigger is compiled
160162
void release(thread_db*); // Try to free trigger request
161163

@@ -191,21 +193,25 @@ class TrigVector : public Firebird::ObjectsArray<Trigger>
191193
useCount(0)
192194
{ }
193195

194-
void addRef() const
196+
void addRef()
195197
{
196198
++useCount;
197199
}
198200

199-
void release() const;
200-
void release(thread_db* tdbb) const;
201+
bool hasActive() const;
202+
203+
void decompile(thread_db* tdbb);
204+
205+
void release();
206+
void release(thread_db* tdbb);
201207

202208
~TrigVector()
203209
{
204210
fb_assert(useCount.value() == 0);
205211
}
206212

207213
private:
208-
mutable Firebird::AtomicCounter useCount;
214+
Firebird::AtomicCounter useCount;
209215
};
210216

211217

0 commit comments

Comments
 (0)