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, 12 deletions

diff --git a/src/backend/port/win32_latch.c b/src/backend/port/win32_latch.c
index 4bcf7b7a8f3..3da5085a1a8 100644
--- a/src/backend/port/win32_latch.c
+++ b/src/backend/port/win32_latch.c
@@ -1,9 +1,10 @@
 /*-------------------------------------------------------------------------
  *
  * win32_latch.c
- *	  Windows implementation of latches.
+ *	  Routines for inter-process latches
  *
- * See unix_latch.c for information on usage.
+ * See unix_latch.c for header comments for the exported functions;
+ * the API presented here is supposed to be the same as there.
  *
  * The Windows implementation uses Windows events that are inherited by
  * all postmaster child processes.
@@ -23,7 +24,6 @@
 #include <unistd.h>
 
 #include "miscadmin.h"
-#include "replication/walsender.h"
 #include "storage/latch.h"
 #include "storage/shmem.h"
 
@@ -88,7 +88,7 @@ WaitLatch(volatile Latch *latch, long timeout)
 }
 
 int
-WaitLatchOrSocket(volatile Latch *latch, SOCKET sock, bool forRead,
+WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead,
 				  bool forWrite, long timeout)
 {
 	DWORD		rc;
@@ -98,6 +98,9 @@ WaitLatchOrSocket(volatile Latch *latch, SOCKET sock, bool forRead,
 	int			numevents;
 	int			result = 0;
 
+	if (latch->owner_pid != MyProcPid)
+		elog(ERROR, "cannot wait on a latch owned by another process");
+
 	latchevent = latch->event;
 
 	events[0] = latchevent;
@@ -187,15 +190,10 @@ SetLatch(volatile Latch *latch)
 
 	/*
 	 * See if anyone's waiting for the latch. It can be the current process if
-	 * we're in a signal handler. Use a local variable here in case the latch
-	 * is just disowned between the test and the SetEvent call, and event
-	 * field set to NULL.
+	 * we're in a signal handler.
 	 *
-	 * Fetch handle field only once, in case the owner simultaneously disowns
-	 * the latch and clears handle. This assumes that HANDLE is atomic, which
-	 * isn't guaranteed to be true! In practice, it should be, and in the
-	 * worst case we end up calling SetEvent with a bogus handle, and SetEvent
-	 * will return an error with no harm done.
+	 * Use a local variable here just in case somebody changes the event field
+	 * concurrently (which really should not happen).
 	 */
 	handle = latch->event;
 	if (handle)
@@ -212,5 +210,8 @@ SetLatch(volatile Latch *latch)
 void
 ResetLatch(volatile Latch *latch)
 {
+	/* Only the owner should reset the latch */
+	Assert(latch->owner_pid == MyProcPid);
+
 	latch->is_set = false;
 }