Fix self-deadlock when acquiring a shared SyncObject latch

src/common/classes/SyncObject.cpp

@@ -35,6 +35,7 @@
 
 #include "SyncObject.h"
 #include "Synchronize.h"
+#include "../jrd/err_proto.h"
 
 namespace Firebird {
 
@@ -91,6 +92,13 @@ bool SyncObject::lock(Sync* sync, SyncType type, const char* from, int timeOut)
 
 		thread = ThreadSync::findThread();
 		fb_assert(thread);
+
+		if (thread == exclusiveThread)
+		{
+			--waiters;
+			mutex.leave();
+			ERR_post(Arg::Gds(isc_sync_object_self_deadlock));
+		}
 	}
 	else
 	{

src/dsql/StmtNodes.cpp

@@ -44,6 +44,7 @@
 #include "../jrd/trace/TraceManager.h"
 #include "../jrd/trace/TraceJrdHelpers.h"
 #include "../jrd/cmp_proto.h"
+#include "../jrd/cch_proto.h"
 #include "../jrd/dfw_proto.h"
 #include "../jrd/dpm_proto.h"
 #include "../jrd/evl_proto.h"
@@ -2753,7 +2754,19 @@ const StmtNode* EraseNode::erase(thread_db* tdbb, Request* request, WhichTrigger
 		// setting req_update_conflict flag) so re-fetch should see new data.
 		// b) record is locked by another transaction and should be skipped.
 
-		if (!VIO_erase(tdbb, rpb, transaction))
+		bool erased;
+
+		try
+		{
+			erased = VIO_erase(tdbb, rpb, transaction);
+		}
+		catch (const Exception&)
+		{
+			CCH_unwind(tdbb, false);
+			throw;
+		}
+
+		if (!erased)
 		{
 			// Record was not deleted, flow control should be passed to the parent
 			// ForNode. Note, If RETURNING clause was specified and SKIP LOCKED was
@@ -7132,7 +7145,19 @@ const StmtNode* ModifyNode::modify(thread_db* tdbb, Request* request, WhichTrigg
 					// setting req_update_conflict flag) so re-fetch should see new data.
 					// b) record is locked by another transaction and should be skipped.
 
-					if (!VIO_modify(tdbb, orgRpb, newRpb, transaction))
+					bool modified;
+
+					try
+					{
+						modified = VIO_modify(tdbb, orgRpb, newRpb, transaction);
+					}
+					catch (const Exception&)
+					{
+						CCH_unwind(tdbb, false);
+						throw;
+					}
+
+					if (!modified)
 					{
 						if (!skipLocked)
 						{
@@ -8215,7 +8240,16 @@ const StmtNode* StoreNode::store(thread_db* tdbb, Request* request, WhichTrigger
 					VirtualTable::store(tdbb, rpb);
 				else if (!relation->rel_view_rse)
 				{
-					VIO_store(tdbb, rpb, transaction);
+					try
+					{
+						VIO_store(tdbb, rpb, transaction);
+					}
+					catch (const Exception&)
+					{
+						CCH_unwind(tdbb, false);
+						throw;
+					}
+
 					IDX_store(tdbb, rpb, transaction);
 					REPL_store(tdbb, rpb, transaction);
 				}

src/include/firebird/impl/msg/jrd.h

@@ -979,3 +979,5 @@ FB_IMPL_MSG(JRD, 998, no_user_att_while_restore, -901, "HY", "000", "User attach
 FB_IMPL_MSG(JRD, 1005, update_overwrite, -901, "27", "000", "UPDATE will overwrite changes made by the trigger or by the another UPDATE in the same cursor")
 // Codes 1006..1015 are used in v6
 FB_IMPL_MSG(JRD, 1016, temp_space_invalid_pos, -901, "HY", "000", "Invalid position to read/write in a temporary file (positon: @1, size: @2)")
+// Codes 1017..1022 are used in v6
+FB_IMPL_MSG(JRD, 1023, sync_object_self_deadlock, -901, "HY", "000", "Acquire a SYNC_SHARED latch on a SYNC_EXCLUSIVE latch that is already held by current thread")

src/include/gen/Firebird.pas

@@ -5766,6 +5766,7 @@ 	IProfilerStatsImpl = class(IProfilerStats)
 	 isc_no_user_att_while_restore = 335545318;
 	 isc_update_overwrite = 335545325;
 	 isc_temp_space_invalid_pos = 335545336;
+	 isc_sync_object_self_deadlock = 335545343;
 	 isc_gfix_db_name = 335740929;
 	 isc_gfix_invalid_sw = 335740930;
 	 isc_gfix_incmp_sw = 335740932;

src/jrd/Savepoint.cpp

@@ -472,6 +472,7 @@ Savepoint* Savepoint::rollback(thread_db* tdbb, Savepoint* prior, bool preserveL
 		tdbb->setTransaction(old_tran);
 		m_transaction->tra_flags |= TRA_invalidated;
 		error.prepend(Arg::Gds(isc_savepoint_backout_err));
+		CCH_unwind(tdbb, false);
 		error.raise();
 	}
 

src/jrd/cch.cpp

@@ -2033,6 +2033,16 @@ void CCH_release(thread_db* tdbb, WIN* window, const bool release_tail)
  **************************************/
 	SET_TDBB(tdbb);
 
+	// If TDBB_cache_unwound is set, return here to prevent
+	// changing bdb_flags below because it can be dangerous
+	// in a concurrent environment.
+	if (tdbb->tdbb_flags & TDBB_cache_unwound)
+	{
+		fb_assert(tdbb->tdbb_bdbs.isEmpty());
+		window->win_bdb = NULL;
+		return;
+	}
+
 	BufferDesc* const bdb = window->win_bdb;
 	BLKCHK(bdb, type_bdb);
 

src/jrd/replication/Applier.cpp

@@ -1296,7 +1296,15 @@ void Applier::doInsert(thread_db* tdbb, record_param* rpb, jrd_tra* transaction)
 	// This allows to use RDB$RECORD_VERSION in indices.
 	rpb->rpb_record->setTransactionNumber(transaction->tra_number);
 
-	VIO_store(tdbb, rpb, transaction);
+	try
+	{
+		VIO_store(tdbb, rpb, transaction);
+	}
+	catch (const Exception&)
+	{
+		CCH_unwind(tdbb, false);
+		throw;
+	}
 	IDX_store(tdbb, rpb, transaction);
 	if (m_enableCascade)
 		REPL_store(tdbb, rpb, transaction);
@@ -1396,7 +1404,15 @@ void Applier::doUpdate(thread_db* tdbb, record_param* orgRpb, record_param* newR
 	// This allows to use NEW.RDB$RECORD_VERSION in indices.
 	newRpb->rpb_record->setTransactionNumber(transaction->tra_number);
 
-	VIO_modify(tdbb, orgRpb, newRpb, transaction);
+	try
+	{
+		VIO_modify(tdbb, orgRpb, newRpb, transaction);
+	}
+	catch (const Exception&)
+	{
+		CCH_unwind(tdbb, false);
+		throw;
+	}
 	IDX_modify(tdbb, orgRpb, newRpb, transaction);
 	if (m_enableCascade)
 		REPL_modify(tdbb, orgRpb, newRpb, transaction);
@@ -1410,7 +1426,16 @@ void Applier::doDelete(thread_db* tdbb, record_param* rpb, jrd_tra* transaction)
 
 	Savepoint::ChangeMarker marker(transaction->tra_save_point);
 
-	VIO_erase(tdbb, rpb, transaction);
+	try
+	{
+		VIO_erase(tdbb, rpb, transaction);
+	}
+	catch (const Exception&)
+	{
+		CCH_unwind(tdbb, false);
+		throw;
+	}
+
 	if (m_enableCascade)
 		REPL_erase(tdbb, rpb, transaction);
 }