From 6760a4d4029121981bf3ec24847ddfbacecc070d Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 9 Aug 2011 15:30:51 -0400 Subject: 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. --- src/backend/port/win32_latch.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) (limited to 'src/backend/port/win32_latch.c') 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 #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; } -- cgit v1.2.3