From 8a7e8d68d6b30e0d180b23a88f1e486594f8cb5f Mon Sep 17 00:00:00 2001
From: Michael Zhilin <mizhka@FreeBSD.org>
Date: Sun, 31 Dec 2023 17:48:20 +0300
Subject: [PATCH v1] Avoid deadlock and concurrency during orphan temp table
 removal

If temp tables contains autogenerated objects (for example, sequences),
then deadlock with client worker and autovacuum worker is possible at
the moment of orphan temp table removal after unexpected server restart.

The fix idea is to avoid concurrency by conditional locking of namespace
in autovacuum worker.
---
 src/backend/postmaster/autovacuum.c | 20 +++++++++++++++
 src/backend/storage/lmgr/lmgr.c     | 38 +++++++++++++++++++++++++++++
 src/include/storage/lmgr.h          |  2 ++
 3 files changed, 60 insertions(+)

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 601834d4b4e..324bb30a404 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -76,6 +76,7 @@
 #include "catalog/dependency.h"
 #include "catalog/namespace.h"
 #include "catalog/pg_database.h"
+#include "catalog/pg_namespace.h"
 #include "commands/dbcommands.h"
 #include "commands/vacuum.h"
 #include "lib/ilist.h"
@@ -2261,6 +2262,19 @@ do_autovacuum(void)
 			continue;
 		}
 
+		/*
+		 * Try to lock the table namespace.  As explained above, we try to avoid
+		 * concurrency on table removals. If backend or another autovacuum
+		 * worker tries to drop orphan temp tables, let it do its job.
+		 */
+		if (!ConditionalLockDatabaseObject(NamespaceRelationId,
+										   classForm->relnamespace, 0,
+										   AccessShareLock))
+		{
+			UnlockRelationOid(relid, AccessExclusiveLock);
+			continue;
+		}
+
 		/* OK, let's delete it */
 		ereport(LOG,
 				(errmsg("autovacuum: dropping orphan temp table \"%s.%s.%s\"",
@@ -2287,6 +2301,12 @@ do_autovacuum(void)
 		MemoryContextSwitchTo(AutovacMemCxt);
 	}
 
+	if (orphan_oids != NIL) {
+		ereport(LOG,
+				(errmsg("autovacuum: dropping orphan temp tables completed")));
+	}
+
+
 	/*
 	 * Create a buffer access strategy object for VACUUM to use.  We want to
 	 * use the same one across all the vacuum operations we perform, since the
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 1043068bac7..0aa663346a4 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -1019,6 +1019,44 @@ LockDatabaseObject(Oid classid, Oid objid, uint16 objsubid,
 	AcceptInvalidationMessages();
 }
 
+/*
+ *		ConditionalLockDatabaseObject
+ *
+ * As above, but only lock if we can get the lock without blocking.
+ * Returns true iff the lock was acquired.
+ */
+bool
+ConditionalLockDatabaseObject(Oid classid, Oid objid, uint16 objsubid,
+		   LOCKMODE lockmode)
+{
+	LOCKTAG		tag;
+	LOCALLOCK  *locallock;
+	LockAcquireResult res;
+
+	SET_LOCKTAG_OBJECT(tag,
+					   MyDatabaseId,
+					   classid,
+					   objid,
+					   objsubid);
+
+	res = LockAcquireExtended(&tag, lockmode, false, true, true, &locallock);
+
+	if (res == LOCKACQUIRE_NOT_AVAIL)
+		return false;
+
+	/*
+	 * Now that we have the lock, check for invalidation messages; see notes
+	 * in LockRelationOid.
+	 */
+	if (res != LOCKACQUIRE_ALREADY_CLEAR)
+	{
+		AcceptInvalidationMessages();
+		MarkLockClear(locallock);
+	}
+
+	return true;
+}
+
 /*
  *		UnlockDatabaseObject
  */
diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h
index be1d2c99a95..2c0c1f346fa 100644
--- a/src/include/storage/lmgr.h
+++ b/src/include/storage/lmgr.h
@@ -93,6 +93,8 @@ extern void SpeculativeInsertionWait(TransactionId xid, uint32 token);
 /* Lock a general object (other than a relation) of the current database */
 extern void LockDatabaseObject(Oid classid, Oid objid, uint16 objsubid,
 							   LOCKMODE lockmode);
+extern bool ConditionalLockDatabaseObject(Oid classid, Oid objid,
+										  uint16 objsubid, LOCKMODE lockmode);
 extern void UnlockDatabaseObject(Oid classid, Oid objid, uint16 objsubid,
 								 LOCKMODE lockmode);
 
-- 
2.43.0