From 83c97a2060e1b4d57ef4ff5b27da5b57b4d7118e Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Tue, 23 Jul 2024 07:56:10 -0300 Subject: [PATCH] Fix #8185 - SIGSEGV with WHERE CURRENT OF statement with statement cache turned on. --- src/dsql/DsqlRequests.cpp | 5 ++++- src/dsql/DsqlStatementCache.cpp | 34 ++++++++++++++++++++++++++++++--- src/dsql/DsqlStatementCache.h | 1 + src/dsql/DsqlStatements.cpp | 6 ++---- src/dsql/DsqlStatements.h | 1 + 5 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/dsql/DsqlRequests.cpp b/src/dsql/DsqlRequests.cpp index b9d0b2f4cac..cb68720f7e9 100644 --- a/src/dsql/DsqlRequests.cpp +++ b/src/dsql/DsqlRequests.cpp @@ -23,7 +23,7 @@ #include "../dsql/DsqlRequests.h" #include "../dsql/dsql.h" #include "../dsql/DsqlBatch.h" -///#include "../dsql/DsqlStatementCache.h" +#include "../dsql/DsqlStatementCache.h" #include "../dsql/Nodes.h" #include "../jrd/Statement.h" #include "../jrd/req.h" @@ -177,6 +177,9 @@ void DsqlRequest::destroy(thread_db* tdbb, DsqlRequest* dsqlRequest) { childStatement->addFlags(DsqlStatement::FLAG_ORPHAN); childStatement->setParentRequest(nullptr); + childStatement->setParentDbKey(nullptr); + childStatement->setParentRecVersion(nullptr); + dsqlRequest->req_dbb->dbb_statement_cache->removeStatement(tdbb, childStatement); // hvlad: lines below is commented out as // - child is already unlinked from its parent request diff --git a/src/dsql/DsqlStatementCache.cpp b/src/dsql/DsqlStatementCache.cpp index 0f13068507b..2ba98386a22 100644 --- a/src/dsql/DsqlStatementCache.cpp +++ b/src/dsql/DsqlStatementCache.cpp @@ -156,20 +156,44 @@ void DsqlStatementCache::putStatement(thread_db* tdbb, const string& text, USHOR #endif } +void DsqlStatementCache::removeStatement(thread_db* tdbb, DsqlStatement* statement) +{ + if (const auto cacheKey = statement->getCacheKey()) + { + if (const auto entryPtr = map.get(cacheKey)) + { + const auto entry = *entryPtr; + + entry->dsqlStatement->resetCacheKey(); + + if (entry->active) + { + entry->dsqlStatement->addRef(); + activeStatementList.erase(entry); + } + else + { + inactiveStatementList.erase(entry); + cacheSize -= entry->size; + } + + map.remove(entry->key); + } + } +} + void DsqlStatementCache::statementGoingInactive(Firebird::RefStrPtr& key) { const auto entryPtr = map.get(key); if (!entryPtr) - { - fb_assert(false); return; - } const auto entry = *entryPtr; fb_assert(entry->active); entry->active = false; + entry->dsqlStatement->addRef(); entry->size = entry->dsqlStatement->getSize(); // update size inactiveStatementList.splice(inactiveStatementList.end(), activeStatementList, entry); @@ -192,6 +216,9 @@ void DsqlStatementCache::purge(thread_db* tdbb, bool releaseLock) entry.dsqlStatement->resetCacheKey(); } + for (auto& entry : inactiveStatementList) + entry.dsqlStatement->resetCacheKey(); + map.clear(); activeStatementList.clear(); inactiveStatementList.clear(); @@ -273,6 +300,7 @@ void DsqlStatementCache::shrink() while (cacheSize > maxCacheSize && !inactiveStatementList.isEmpty()) { const auto& front = inactiveStatementList.front(); + front.dsqlStatement->resetCacheKey(); map.remove(front.key); cacheSize -= front.size; inactiveStatementList.erase(inactiveStatementList.begin()); diff --git a/src/dsql/DsqlStatementCache.h b/src/dsql/DsqlStatementCache.h index b7d57596683..8f31a6411ab 100644 --- a/src/dsql/DsqlStatementCache.h +++ b/src/dsql/DsqlStatementCache.h @@ -106,6 +106,7 @@ class DsqlStatementCache final : public Firebird::PermanentStorage void putStatement(thread_db* tdbb, const Firebird::string& text, USHORT clientDialect, bool isInternalRequest, Firebird::RefPtr dsqlStatement); + void removeStatement(thread_db* tdbb, DsqlStatement* statement); void statementGoingInactive(Firebird::RefStrPtr& key); void purge(thread_db* tdbb, bool releaseLock); diff --git a/src/dsql/DsqlStatements.cpp b/src/dsql/DsqlStatements.cpp index cdba88ec60b..47117b26a68 100644 --- a/src/dsql/DsqlStatements.cpp +++ b/src/dsql/DsqlStatements.cpp @@ -65,10 +65,8 @@ int DsqlStatement::release() { if (cacheKey) { - refCnt = ++refCounter; - auto key = cacheKey; - cacheKey = nullptr; - dsqlAttachment->dbb_statement_cache->statementGoingInactive(key); + dsqlAttachment->dbb_statement_cache->statementGoingInactive(cacheKey); + refCnt = refCounter; } else { diff --git a/src/dsql/DsqlStatements.h b/src/dsql/DsqlStatements.h index 0180d92e29f..1ab48fc9862 100644 --- a/src/dsql/DsqlStatements.h +++ b/src/dsql/DsqlStatements.h @@ -135,6 +135,7 @@ class DsqlStatement : public Firebird::PermanentStorage const dsql_par* getEof() const { return eof; } void setEof(dsql_par* value) { eof = value; } + Firebird::RefStrPtr getCacheKey() { return cacheKey; } void setCacheKey(Firebird::RefStrPtr& value) { cacheKey = value; } void resetCacheKey() { cacheKey = nullptr; }