postgres.git / summary / log / commit / refs

commit    469fa9ad6d0564b7252e89726533c0b7ccb2c4ec
Author:   Alvaro Herrera <alvherre@alvh.no-ip.org>
Date:     Tue Jan 09 18:54:38 2018 +0000
First released in: 11.0, 9.3.21, 9.4.16, 9.5.11, 9.6.7, 10.2

    Change some bogus PageGetLSN calls to BufferGetLSNAtomic
    
    As src/backend/access/transam/README says, PageGetLSN may only be called
    by processes holding either exclusive lock on buffer, or a shared lock
    on buffer plus buffer header lock.  Therefore any place that only holds
    a shared buffer lock must use BufferGetLSNAtomic instead of PageGetLSN,
    which internally obtains buffer header lock prior to reading the LSN.
    
    A few callsites failed to comply with this rule.  This was detected by
    running all tests under a new (not committed) assertion that verifies
    PageGetLSN locking contract.  All but one of the callsites that failed
    the assertion are fixed by this patch.  Remaining callsites were
    inspected manually and determined not to need any change.
    
    The exception (unfixed callsite) is in TestForOldSnapshot, which only
    has a Page argument, making it impossible to access the corresponding
    Buffer from it.  Fixing that seems a much larger patch that will have to
    be done separately; and that's just as well, since it was only
    introduced in 9.6 and other bugs are much older.
    
    Some of these bugs are ancient; backpatch all the way back to 9.3.
    
    Authors: Jacob Champion, Asim Praveen, Ashwin Agrawal
    Reviewed-by: Michaël Paquier
    Discussion: https://postgr.es/m/CABAq_6GXgQDVu3u12mK9O5Xt5abBZWQ0V40LZCE+oUf95XyNFg@mail.gmail.com


 src/backend/access/gist/gist.c       | 5 +++--
 src/backend/access/gist/gistvacuum.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 14c7856dde6..a55a9d8ccdd 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -558,7 +558,8 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, GISTSTATE *giststate)
 		}
 
 		stack->page = (Page) BufferGetPage(stack->buffer);
-		stack->lsn = PageGetLSN(stack->page);
+		stack->lsn = xlocked ?
+			PageGetLSN(stack->page) : BufferGetLSNAtomic(stack->buffer);
 		Assert(!RelationNeedsWAL(state.r) || !XLogRecPtrIsInvalid(stack->lsn));
 
 		/*
@@ -808,7 +809,7 @@ gistFindPath(Relation r, BlockNumber child, OffsetNumber *downlinkoffnum)
 			break;
 		}
 
-		top->lsn = PageGetLSN(page);
+		top->lsn = BufferGetLSNAtomic(buffer);
 
 		/*
 		 * If F_FOLLOW_RIGHT is set, the page to the right doesn't have a
diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c
index dc32c9d5292..8ecb12239de 100644
--- a/src/backend/access/gist/gistvacuum.c
+++ b/src/backend/access/gist/gistvacuum.c
@@ -257,7 +257,7 @@ gistbulkdelete(PG_FUNCTION_ARGS)
 
 				ptr = (GistBDItem *) palloc(sizeof(GistBDItem));
 				ptr->blkno = ItemPointerGetBlockNumber(&(idxtuple->t_tid));
-				ptr->parentlsn = PageGetLSN(page);
+				ptr->parentlsn = BufferGetLSNAtomic(buffer);
 				ptr->next = stack->next;
 				stack->next = ptr;
 


[parent: 5404145c5ad4]