postgres.git / summary / log / commit / refs

commit    4c853646ae063d6290fc535e3aa80bc0ac69f012
Author:   Andres Freund <andres@anarazel.de>
Date:     Fri Dec 19 13:29:52 2014 +0000
First released in: 9.5.0, 9.0.19, 9.1.15, 9.2.10, 9.3.6, 9.4.1

    Prevent potentially hazardous compiler/cpu reordering during lwlock release.
    
    In LWLockRelease() (and in 9.4+ LWLockUpdateVar()) we release enqueued
    waiters using PGSemaphoreUnlock(). As there are other sources of such
    unlocks backends only wake up if MyProc->lwWaiting is set to false;
    which is only done in the aforementioned functions.
    
    Before this commit there were dangers because the store to lwWaitLink
    could become visible before the store to lwWaitLink. This could both
    happen due to compiler reordering (on most compilers) and on some
    platforms due to the CPU reordering stores.
    
    The possible consequence of this is that a backend stops waiting
    before lwWaitLink is set to NULL. If that backend then tries to
    acquire another lock and has to wait there the list could become
    corrupted once the lwWaitLink store is finally performed.
    
    Add a write memory barrier to prevent that issue.
    
    Unfortunately the barrier support has been only added in 9.2. Given
    that the issue has not knowingly been observed in praxis it seems
    sufficient to prohibit compiler reordering using volatile for 9.0 and
    9.1. Actual problems due to compiler reordering are more likely
    anyway.
    
    Discussion: 20140210134625.GA15246@awork2.anarazel.de


 src/backend/storage/lmgr/lwlock.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 88d9cbec215..196b1110326 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -35,6 +35,7 @@
 #include "miscadmin.h"
 #include "pg_trace.h"
 #include "replication/slot.h"
+#include "storage/barrier.h"
 #include "storage/ipc.h"
 #include "storage/predicate.h"
 #include "storage/proc.h"
@@ -1102,6 +1103,8 @@ LWLockUpdateVar(LWLock *l, uint64 *valptr, uint64 val)
 		proc = head;
 		head = proc->lwWaitLink;
 		proc->lwWaitLink = NULL;
+		/* check comment in LWLockRelease() about this barrier */
+		pg_write_barrier();
 		proc->lwWaiting = false;
 		PGSemaphoreUnlock(&proc->sem);
 	}
@@ -1222,6 +1225,17 @@ LWLockRelease(LWLock *l)
 		proc = head;
 		head = proc->lwWaitLink;
 		proc->lwWaitLink = NULL;
+		/*
+		 * Guarantee that lwWaiting being unset only becomes visible once the
+		 * unlink from the link has completed. Otherwise the target backend
+		 * could be woken up for other reason and enqueue for a new lock - if
+		 * that happens before the list unlink happens, the list would end up
+		 * being corrupted.
+		 *
+		 * The barrier pairs with the SpinLockAcquire() when enqueing for
+		 * another lock.
+		 */
+		pg_write_barrier();
 		proc->lwWaiting = false;
 		PGSemaphoreUnlock(&proc->sem);
 	}


[parent: 2b8f74d79722]