Fix check for conflicting session- vs transaction-level locks.

We have an implementation restriction that PREPARE TRANSACTION can't
handle cases where both session-lifespan and transaction-lifespan locks
are held on the same lockable object.  (That's because we'd otherwise
need to acquire a new PROCLOCK entry during post-prepare cleanup, which
is an operation that might fail.  The situation can only arise with odd
usages of advisory locks, so removing the restriction is probably not
worth the amount of effort it would take.)  AtPrepare_Locks attempted
to enforce this, but its logic was many bricks shy of a load, because
it only detected cases where the session and transaction locks had the
same lockmode.  Locks of different modes on the same object would lead
to the rather unhelpful message "PANIC: we seem to have dropped a bit
somewhere".

To fix, build a transient hashtable with one entry per locktag,
not one per locktag + mode, and use that to detect conflicts.

Per bug #17122 from Alexander Pyhalov.  This bug is ancient,
so back-patch to all supported branches.

Discussion: https://postgr.es/m/17122-04f3c32098a62233@postgresql.org

diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 1c5ab8c0d97b4c585be4bed3dc3c4e9982d8129f..8d693546d482bed4881b8f648119f9e36d20a211 100644 (file)
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -3088,6 +3088,102 @@ LockRefindAndRelease(LockMethod lockMethodTable, PGPROC *proc,
    }
 }
 
+/*
+ * CheckForSessionAndXactLocks
+ *     Check to see if transaction holds both session-level and xact-level
+ *     locks on the same object; if so, throw an error.
+ *
+ * If we have both session- and transaction-level locks on the same object,
+ * PREPARE TRANSACTION must fail.  This should never happen with regular
+ * locks, since we only take those at session level in some special operations
+ * like VACUUM.  It's possible to hit this with advisory locks, though.
+ *
+ * It would be nice if we could keep the session hold and give away the
+ * transactional hold to the prepared xact.  However, that would require two
+ * PROCLOCK objects, and we cannot be sure that another PROCLOCK will be
+ * available when it comes time for PostPrepare_Locks to do the deed.
+ * So for now, we error out while we can still do so safely.
+ *
+ * Since the LOCALLOCK table stores a separate entry for each lockmode,
+ * we can't implement this check by examining LOCALLOCK entries in isolation.
+ * We must build a transient hashtable that is indexed by locktag only.
+ */
+static void
+CheckForSessionAndXactLocks(void)
+{
+   typedef struct
+   {
+       LOCKTAG     lock;       /* identifies the lockable object */
+       bool        sessLock;   /* is any lockmode held at session level? */
+       bool        xactLock;   /* is any lockmode held at xact level? */
+   } PerLockTagEntry;
+
+   HASHCTL     hash_ctl;
+   HTAB       *lockhtab;
+   HASH_SEQ_STATUS status;
+   LOCALLOCK  *locallock;
+
+   /* Create a local hash table keyed by LOCKTAG only */
+   hash_ctl.keysize = sizeof(LOCKTAG);
+   hash_ctl.entrysize = sizeof(PerLockTagEntry);
+   hash_ctl.hcxt = CurrentMemoryContext;
+
+   lockhtab = hash_create("CheckForSessionAndXactLocks table",
+                          256, /* arbitrary initial size */
+                          &hash_ctl,
+                          HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
+
+   /* Scan local lock table to find entries for each LOCKTAG */
+   hash_seq_init(&status, LockMethodLocalHash);
+
+   while ((locallock = (LOCALLOCK *) hash_seq_search(&status)) != NULL)
+   {
+       LOCALLOCKOWNER *lockOwners = locallock->lockOwners;
+       PerLockTagEntry *hentry;
+       bool        found;
+       int         i;
+
+       /*
+        * Ignore VXID locks.  We don't want those to be held by prepared
+        * transactions, since they aren't meaningful after a restart.
+        */
+       if (locallock->tag.lock.locktag_type == LOCKTAG_VIRTUALTRANSACTION)
+           continue;
+
+       /* Ignore it if we don't actually hold the lock */
+       if (locallock->nLocks <= 0)
+           continue;
+
+       /* Otherwise, find or make an entry in lockhtab */
+       hentry = (PerLockTagEntry *) hash_search(lockhtab,
+                                                (void *) &locallock->tag.lock,
+                                                HASH_ENTER, &found);
+       if (!found)             /* initialize, if newly created */
+           hentry->sessLock = hentry->xactLock = false;
+
+       /* Scan to see if we hold lock at session or xact level or both */
+       for (i = locallock->numLockOwners - 1; i >= 0; i--)
+       {
+           if (lockOwners[i].owner == NULL)
+               hentry->sessLock = true;
+           else
+               hentry->xactLock = true;
+       }
+
+       /*
+        * We can throw error immediately when we see both types of locks; no
+        * need to wait around to see if there are more violations.
+        */
+       if (hentry->sessLock && hentry->xactLock)
+           ereport(ERROR,
+                   (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                    errmsg("cannot PREPARE while holding both session-level and transaction-level locks on the same object")));
+   }
+
+   /* Success, so clean up */
+   hash_destroy(lockhtab);
+}
+
 /*
  * AtPrepare_Locks
  *     Do the preparatory work for a PREPARE: make 2PC state file records
@@ -3095,11 +3191,10 @@ LockRefindAndRelease(LockMethod lockMethodTable, PGPROC *proc,
  *
  * Session-level locks are ignored, as are VXID locks.
  *
- * There are some special cases that we error out on: we can't be holding any
- * locks at both session and transaction level (since we must either keep or
- * give away the PROCLOCK object), and we can't be holding any locks on
- * temporary objects (since that would mess up the current backend if it tries
- * to exit before the prepared xact is committed).
+ * For the most part, we don't need to touch shared memory for this ---
+ * all the necessary state information is in the locallock table.
+ * Fast-path locks are an exception, however: we move any such locks to
+ * the main table before allowing PREPARE TRANSACTION to succeed.
  */
 void
 AtPrepare_Locks(void)
@@ -3107,12 +3202,10 @@ AtPrepare_Locks(void)
    HASH_SEQ_STATUS status;
    LOCALLOCK  *locallock;
 
-   /*
-    * For the most part, we don't need to touch shared memory for this ---
-    * all the necessary state information is in the locallock table.
-    * Fast-path locks are an exception, however: we move any such locks to
-    * the main table before allowing PREPARE TRANSACTION to succeed.
-    */
+   /* First, verify there aren't locks of both xact and session level */
+   CheckForSessionAndXactLocks();
+
+   /* Now do the per-locallock cleanup work */
    hash_seq_init(&status, LockMethodLocalHash);
 
    while ((locallock = (LOCALLOCK *) hash_seq_search(&status)) != NULL)
@@ -3148,19 +3241,7 @@ AtPrepare_Locks(void)
        if (!haveXactLock)
            continue;
 
-       /*
-        * If we have both session- and transaction-level locks, fail.  This
-        * should never happen with regular locks, since we only take those at
-        * session level in some special operations like VACUUM.  It's
-        * possible to hit this with advisory locks, though.
-        *
-        * It would be nice if we could keep the session hold and give away
-        * the transactional hold to the prepared xact.  However, that would
-        * require two PROCLOCK objects, and we cannot be sure that another
-        * PROCLOCK will be available when it comes time for PostPrepare_Locks
-        * to do the deed.  So for now, we error out while we can still do so
-        * safely.
-        */
+       /* This can't happen, because we already checked it */
        if (haveSessionLock)
            ereport(ERROR,
                    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),

diff --git a/src/test/regress/expected/prepared_xacts.out b/src/test/regress/expected/prepared_xacts.out
index 01cf87e9d7c085af937708d6a610c1a5c2a08131..773648e6901c8786b6f24fba7fd16c4c75d6160c 100644 (file)
--- a/src/test/regress/expected/prepared_xacts.out
+++ b/src/test/regress/expected/prepared_xacts.out
@@ -151,6 +151,22 @@ SELECT gid FROM pg_prepared_xacts;
 
 -- Clean up
 DROP TABLE pxtest1;
+-- Test detection of session-level and xact-level locks on same object
+BEGIN;
+SELECT pg_advisory_lock(1);
+ pg_advisory_lock 
+------------------
+ 
+(1 row)
+
+SELECT pg_advisory_xact_lock_shared(1);
+ pg_advisory_xact_lock_shared 
+------------------------------
+ 
+(1 row)
+
+PREPARE TRANSACTION 'foo6';  -- fails
+ERROR:  cannot PREPARE while holding both session-level and transaction-level locks on the same object
 -- Test subtransactions
 BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
   CREATE TABLE pxtest2 (a int);

diff --git a/src/test/regress/expected/prepared_xacts_1.out b/src/test/regress/expected/prepared_xacts_1.out
index 0857d259e07c3601898c6e4bcc55061590affe09..2cd50ad947003260d8ec5b79cb92dbb9fdb36bb8 100644 (file)
--- a/src/test/regress/expected/prepared_xacts_1.out
+++ b/src/test/regress/expected/prepared_xacts_1.out
@@ -153,6 +153,23 @@ SELECT gid FROM pg_prepared_xacts;
 
 -- Clean up
 DROP TABLE pxtest1;
+-- Test detection of session-level and xact-level locks on same object
+BEGIN;
+SELECT pg_advisory_lock(1);
+ pg_advisory_lock 
+------------------
+ 
+(1 row)
+
+SELECT pg_advisory_xact_lock_shared(1);
+ pg_advisory_xact_lock_shared 
+------------------------------
+ 
+(1 row)
+
+PREPARE TRANSACTION 'foo6';  -- fails
+ERROR:  prepared transactions are disabled
+HINT:  Set max_prepared_transactions to a nonzero value.
 -- Test subtransactions
 BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
   CREATE TABLE pxtest2 (a int);

diff --git a/src/test/regress/sql/prepared_xacts.sql b/src/test/regress/sql/prepared_xacts.sql
index d8249a27dc2dc224bcfb432142b18236c0a93192..2f0bb55bb492c489ef67f429f8599fa80aa20c07 100644 (file)
--- a/src/test/regress/sql/prepared_xacts.sql
+++ b/src/test/regress/sql/prepared_xacts.sql
@@ -88,6 +88,12 @@ SELECT gid FROM pg_prepared_xacts;
 -- Clean up
 DROP TABLE pxtest1;
 
+-- Test detection of session-level and xact-level locks on same object
+BEGIN;
+SELECT pg_advisory_lock(1);
+SELECT pg_advisory_xact_lock_shared(1);
+PREPARE TRANSACTION 'foo6';  -- fails
+
 -- Test subtransactions
 BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
   CREATE TABLE pxtest2 (a int);