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/replication/syncrep.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'src/backend/replication/syncrep.c') 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); } /* -- cgit v1.2.3