From ad3e7ebaf80d306605ea58cceaa1d1fa44defc45 Mon Sep 17 00:00:00 2001 From: Ilya Eremin Date: Mon, 8 Apr 2024 14:25:22 +0300 Subject: [PATCH] Fix #7783: Deadlock during index creation when parallel workers are used Deadlock is possible when index deletion and creation are performed in the same transaction and a deleted index is used by a computed field expression. Each worker attachment loads table's metadata, parses the expression and tries to get SR lock on the deleted index. They will never get it because the main thread holds EX lock and waits for worker attachments to complete. The solution is to prevent the optimizer from using deleted indexes by making an attempt to get SR lock with NO_WAIT. --- src/jrd/Statement.cpp | 11 ++----- src/jrd/optimizer/Optimizer.cpp | 54 ++++++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/src/jrd/Statement.cpp b/src/jrd/Statement.cpp index 3aac47c4d17..b5a80c89442 100644 --- a/src/jrd/Statement.cpp +++ b/src/jrd/Statement.cpp @@ -119,15 +119,8 @@ Statement::Statement(thread_db* tdbb, MemoryPool* p, CompilerScratch* csb) case Resource::rsc_index: { - jrd_rel* relation = resource->rsc_rel; - IndexLock* index = CMP_get_index_lock(tdbb, relation, resource->rsc_id); - if (index) - { - ++index->idl_count; - if (index->idl_count == 1) { - LCK_lock(tdbb, index->idl_lock, LCK_SR, LCK_WAIT); - } - } + // No need to lock the index here because it is already + // locked by Optimizer::compileRelation. break; } diff --git a/src/jrd/optimizer/Optimizer.cpp b/src/jrd/optimizer/Optimizer.cpp index 80bdf833498..990c6bf8843 100644 --- a/src/jrd/optimizer/Optimizer.cpp +++ b/src/jrd/optimizer/Optimizer.cpp @@ -1149,6 +1149,38 @@ void Optimizer::compileRelation(StreamType stream) } } + { // scope + ThreadStatusGuard tempStatus(tdbb); + + // Check if some indices are being deleted and don't use them in this case + for (auto idx = idxList.begin(); idx < idxList.end();) + { + IndexLock* idx_lock = CMP_get_index_lock(tdbb, relation, idx->idx_id); + + if (idx_lock) + { + if (idx_lock->idl_count == 1 && idx_lock->idl_lock->lck_logical == LCK_EX) + { + idxList.remove(idx); + continue; + } + else + { + if (idx_lock->idl_count == 0 && !LCK_lock(tdbb, idx_lock->idl_lock, LCK_SR, LCK_NO_WAIT)) + { + idxList.remove(idx); + continue; + } + + fb_assert(idx_lock->idl_lock->lck_logical == LCK_SR); + idx_lock->idl_count++; + } + } + + idx++; + } + } + if (idxList.hasData()) tail->csb_idx = FB_NEW_POOL(getPool()) IndexDescList(getPool(), idxList); @@ -1656,6 +1688,27 @@ void Optimizer::checkIndices() for (const auto compileStream : compileStreams) { const auto tail = &csb->csb_rpt[compileStream]; + const auto relation = tail->csb_relation; + + if (tail->csb_idx && relation) + { + // Release locks of unused indices + for (const auto& idx : *tail->csb_idx) + { + if (!(idx.idx_runtime_flags & idx_used)) + { + IndexLock* index = CMP_get_index_lock(tdbb, relation, idx.idx_id); + + if (index && index->idl_count) + { + --index->idl_count; + + if (!index->idl_count) + LCK_release(tdbb, index->idl_lock); + } + } + } + } const auto plan = tail->csb_plan; if (!plan) @@ -1664,7 +1717,6 @@ void Optimizer::checkIndices() if (plan->type != PlanNode::TYPE_RETRIEVE) continue; - const auto relation = tail->csb_relation; if (!relation) return;