From c58accd70bf3f8b436a66c5e9f0d0a1c1d4d5cc6 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 26 Jan 2015 11:57:33 -0500 Subject: [PATCH] Fix volatile-safety issue in asyncQueueReadAllNotifications(). The "pos" variable is modified within PG_TRY and then referenced within PG_CATCH, so for strict POSIX conformance it must be marked volatile. Superficially the code looked safe because pos's address was taken, which was sufficient to force it into memory ... but it's not sufficient to ensure that the compiler applies updates exactly where the program text says to. The volatility marking has to extend into a couple of subroutines too, but I think that's probably a good thing because the risk of out-of-order updates is mostly in those subroutines not asyncQueueReadAllNotifications() itself. In principle the compiler could have re-ordered operations such that an error could be thrown while "pos" had an incorrect value. It's unclear how real the risk is here, but for safety back-patch to all active branches. --- src/backend/commands/async.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index b945d9de6f..7ece527999 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -369,13 +369,13 @@ static void Exec_UnlistenAllCommit(void); static bool IsListeningOn(const char *channel); static void asyncQueueUnregister(void); static bool asyncQueueIsFull(void); -static bool asyncQueueAdvance(QueuePosition *position, int entryLength); +static bool asyncQueueAdvance(volatile QueuePosition *position, int entryLength); static void asyncQueueNotificationToEntry(Notification *n, AsyncQueueEntry *qe); static ListCell *asyncQueueAddEntries(ListCell *nextNotify); static void asyncQueueFillWarning(void); static bool SignalBackends(void); static void asyncQueueReadAllNotifications(void); -static bool asyncQueueProcessPageEntries(QueuePosition *current, +static bool asyncQueueProcessPageEntries(volatile QueuePosition *current, QueuePosition stop, char *page_buffer); static void asyncQueueAdvanceTail(void); @@ -1202,7 +1202,7 @@ asyncQueueIsFull(void) * returns true, else false. */ static bool -asyncQueueAdvance(QueuePosition *position, int entryLength) +asyncQueueAdvance(volatile QueuePosition *position, int entryLength) { int pageno = QUEUE_POS_PAGE(*position); int offset = QUEUE_POS_OFFSET(*position); @@ -1792,7 +1792,7 @@ DisableNotifyInterrupt(void) static void asyncQueueReadAllNotifications(void) { - QueuePosition pos; + volatile QueuePosition pos; QueuePosition oldpos; QueuePosition head; bool advanceTail; @@ -1952,7 +1952,7 @@ asyncQueueReadAllNotifications(void) * The QueuePosition *current is advanced past all processed messages. */ static bool -asyncQueueProcessPageEntries(QueuePosition *current, +asyncQueueProcessPageEntries(volatile QueuePosition *current, QueuePosition stop, char *page_buffer) {