Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 4c85364

Browse files
committed
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
1 parent 2b8f74d commit 4c85364

File tree

1 file changed

+14
-0
lines changed

1 file changed

+14
-0
lines changed

src/backend/storage/lmgr/lwlock.c

+14
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "miscadmin.h"
3636
#include "pg_trace.h"
3737
#include "replication/slot.h"
38+
#include "storage/barrier.h"
3839
#include "storage/ipc.h"
3940
#include "storage/predicate.h"
4041
#include "storage/proc.h"
@@ -1102,6 +1103,8 @@ LWLockUpdateVar(LWLock *l, uint64 *valptr, uint64 val)
11021103
proc = head;
11031104
head = proc->lwWaitLink;
11041105
proc->lwWaitLink = NULL;
1106+
/* check comment in LWLockRelease() about this barrier */
1107+
pg_write_barrier();
11051108
proc->lwWaiting = false;
11061109
PGSemaphoreUnlock(&proc->sem);
11071110
}
@@ -1222,6 +1225,17 @@ LWLockRelease(LWLock *l)
12221225
proc = head;
12231226
head = proc->lwWaitLink;
12241227
proc->lwWaitLink = NULL;
1228+
/*
1229+
* Guarantee that lwWaiting being unset only becomes visible once the
1230+
* unlink from the link has completed. Otherwise the target backend
1231+
* could be woken up for other reason and enqueue for a new lock - if
1232+
* that happens before the list unlink happens, the list would end up
1233+
* being corrupted.
1234+
*
1235+
* The barrier pairs with the SpinLockAcquire() when enqueing for
1236+
* another lock.
1237+
*/
1238+
pg_write_barrier();
12251239
proc->lwWaiting = false;
12261240
PGSemaphoreUnlock(&proc->sem);
12271241
}

0 commit comments

Comments
 (0)