Documentation improvement and minor code cleanups for the latch facility.

Improve the documentation around weak-memory-ordering risks, and do a pass
of general editorialization on the comments in the latch code.  Make the
Windows latch code more like the Unix latch code where feasible; in
particular provide the same Assert checks in both implementations.
Fix poorly-placed WaitLatch call in syncrep.c.

This patch resolves, for the moment, concerns around weak-memory-ordering
bugs in latch-related code: we have documented the restrictions and checked
that existing calls meet them.  In 9.2 I hope that we will install suitable
memory barrier instructions in SetLatch/ResetLatch, so that their callers
don't need to be quite so careful.

1 files changed, 13 insertions, 7 deletions

diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index 2b52d1616be..7fcbec24400 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -166,13 +166,6 @@ SyncRepWaitForLSN(XLogRecPtr XactCommitLSN)
 	{
 		int			syncRepState;
 
-		/*
-		 * Wait on latch for up to 60 seconds. This allows us to check for
-		 * postmaster death regularly while waiting. Note that timeout here
-		 * does not necessarily release from loop.
-		 */
-		WaitLatch(&MyProc->waitLatch, 60000000L);
-
 		/* Must reset the latch before testing state. */
 		ResetLatch(&MyProc->waitLatch);
 
@@ -184,6 +177,12 @@ SyncRepWaitForLSN(XLogRecPtr XactCommitLSN)
 		 * walsender changes the state to SYNC_REP_WAIT_COMPLETE, it will
 		 * never update it again, so we can't be seeing a stale value in that
 		 * case.
+		 *
+		 * Note: on machines with weak memory ordering, the acquisition of
+		 * the lock is essential to avoid race conditions: we cannot be sure
+		 * the sender's state update has reached main memory until we acquire
+		 * the lock.  We could get rid of this dance if SetLatch/ResetLatch
+		 * contained memory barriers.
 		 */
 		syncRepState = MyProc->syncRepState;
 		if (syncRepState == SYNC_REP_WAITING)
@@ -246,6 +245,13 @@ SyncRepWaitForLSN(XLogRecPtr XactCommitLSN)
 			SyncRepCancelWait();
 			break;
 		}
+
+		/*
+		 * Wait on latch for up to 60 seconds. This allows us to check for
+		 * cancel/die signal or postmaster death regularly while waiting. Note
+		 * that timeout here does not necessarily release from loop.
+		 */
+		WaitLatch(&MyProc->waitLatch, 60000000L);
 	}
 
 	/*