postgres.git / summary / log / commit / refs

commit    333f586372aae764b7ad7e2c975b14fd431ce819
Author:   Andres Freund <andres@anarazel.de>
Date:     Thu Jan 29 21:49:01 2026 +0000

    bufmgr: Allow conditionally locking of already locked buffer
    
    In fcb9c977aa5 I included an assertion in BufferLockConditional() to detect if
    a conditional lock acquisition is done on a buffer that we already have
    locked. The assertion was added in the course of adding other assertions.
    Unfortunately I failed to realize that some of our code relies on such lock
    acquisitions to silently fail. E.g. spgist and nbtree may try to conditionally
    lock an already locked buffer when acquiring a empty buffer.
    
    LWLockAcquireConditional(), which was previously used to implement
    ConditionalLockBuffer(), does not have such an assert.
    
    Instead of just removing the assert, and relying on the lock acquisition to
    fail due to the buffer already locked, this commit changes the behaviour of
    conditional content lock acquisition to fail if the current backend has any
    pre-existing lock on the buffer, even if the lock modes would not
    conflict. The reason for that is that we currently do not have space to track
    multiple lock acquisitions on a single buffer. Allowing multiple locks on the
    same buffer by a backend also seems likely to lead to bugs.
    
    There is only one non-self-exclusive conditional content lock acquisition, in
    GetVictimBuffer(), but it only is used if the target buffer is not pinned and
    thus can't already be locked by the current backend.
    
    Reported-by: Alexander Lakhin <exclusion@gmail.com>
    Discussion: https://postgr.es/m/90bd2cbb-49ce-4092-9f61-5ac2ab782c94@gmail.com


src/backend/storage/buffer/bufmgr.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 6f935648ae9..7241477cac0 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -5895,6 +5895,13 @@ BufferLockUnlock(Buffer buffer, BufferDesc *buf_hdr) /* * Acquire the content lock for the buffer, but only if we don't have to wait. + * + * It is allowed to try to conditionally acquire a lock on a buffer that this + * backend has already locked, but the lock acquisition will always fail, even + * if the new lock acquisition does not conflict with an already held lock + * (e.g. two share locks). This is because we currently do not have space to + * track multiple lock ownerships of the same buffer within one backend. That + * is ok for the current uses of BufferLockConditional(). */ static bool BufferLockConditional(Buffer buffer, BufferDesc *buf_hdr, BufferLockMode mode) @@ -5903,9 +5910,12 @@ BufferLockConditional(Buffer buffer, BufferDesc *buf_hdr, BufferLockMode mode) bool mustwait; /* - * We better not already hold a lock on the buffer. + * As described above, if we're trying to lock a buffer this backend + * already has locked, return false, independent of the existing and + * desired lock level. */ - Assert(entry->data.lockmode == BUFFER_LOCK_UNLOCK); + if (entry->data.lockmode != BUFFER_LOCK_UNLOCK) + return false; /* * Lock out cancel/die interrupts until we exit the code section protected [parent: bd9dfac8b121]