postgres.git / summary / log / commit / refs

commit    fdcee9f1fc12d6d2c20da8da632b2062d2ed6289
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Sun Oct 23 23:14:32 2016 +0000

    Avoid testing tuple visibility without buffer lock.
    
    INSERT ... ON CONFLICT (specifically ExecCheckHeapTupleVisible) contains
    another example of this unsafe coding practice.  It is much harder to get
    a failure out of it than the case fixed in commit 6292c2339, because in
    most scenarios any hint bits that could be set would have already been set
    earlier in the command.  However, Konstantin Knizhnik reported a failure
    with a custom transaction manager, and it's clearly possible to get a
    failure via a race condition in async-commit mode.
    
    For lack of a reproducible example, no regression test case in this
    commit.
    
    I did some testing with Asserts added to tqual.c's functions, and can say
    that running "make check-world" exposed these two bugs and no others.
    The Asserts are messy enough that I've not added them to the code for now.
    
    Report: <57EE93C8.8080504@postgrespro.ru>
    Related-Discussion: <CAO3NbwOycQjt2Oqy2VW-eLTq2M5uGMyHnGm=RNga4mjqcYD7gQ@mail.gmail.com>


 src/backend/executor/nodeModifyTable.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index b056dd9e951..efb0c5e8e5d 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -194,6 +194,11 @@ ExecCheckHeapTupleVisible(EState *estate,
 	if (!IsolationUsesXactSnapshot())
 		return;
 
+	/*
+	 * We need buffer pin and lock to call HeapTupleSatisfiesVisibility.
+	 * Caller should be holding pin, but not lock.
+	 */
+	LockBuffer(buffer, BUFFER_LOCK_SHARE);
 	if (!HeapTupleSatisfiesVisibility(tuple, estate->es_snapshot, buffer))
 	{
 		/*
@@ -207,6 +212,7 @@ ExecCheckHeapTupleVisible(EState *estate,
 					(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
 			 errmsg("could not serialize access due to concurrent update")));
 	}
+	LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
 }
 
 /*


[parent: c4016fcb1fd2]