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
This commit is contained in:
Tom Lane 2021-07-24 18:35:52 -04:00
parent 3779ac62d7
commit 6310809c4a
4 changed files with 144 additions and 24 deletions

View File

@ -3204,6 +3204,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
@ -3211,11 +3307,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)
@ -3223,12 +3318,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)
@ -3264,19 +3357,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),

View File

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

View File

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

View File

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