Backport complete fix for #8185 - SIGSEGV with WHERE CURRENT OF statement with statement cache turned on

Author: asfernandes

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"
@@ -179,6 +179,7 @@ void DsqlRequest::destroy(thread_db* tdbb, DsqlRequest* dsqlRequest)
 		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

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());

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> dsqlStatement);
 
+	void removeStatement(thread_db* tdbb, DsqlStatement* statement);
 	void statementGoingInactive(Firebird::RefStrPtr& key);
 
 	void purge(thread_db* tdbb, bool releaseLock);

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
 		{

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; }
 

src/dsql/dsql.cpp

@@ -639,11 +639,7 @@ static RefPtr<DsqlStatement> prepareStatement(thread_db* tdbb, dsql_dbb* databas
 		if (!isInternalRequest && dsqlStatement->mustBeReplicated())
 			dsqlStatement->setOrgText(text, textLength);
 
-		const bool basedOnCursor =
-				(dsqlStatement->getType() == DsqlStatement::TYPE_UPDATE_CURSOR ||
-				 dsqlStatement->getType() == DsqlStatement::TYPE_DELETE_CURSOR);
-
-		if (isStatementCacheActive && dsqlStatement->isDml() && !basedOnCursor)
+		if (isStatementCacheActive && dsqlStatement->isDml())
 		{
 			database->dbb_statement_cache->putStatement(tdbb,
 				textStr, clientDialect, isInternalRequest, dsqlStatement);