NotificationsYou must be signed in to change notification settings
Fork6
Star31

Commitc58accd

committed

Fix volatile-safety issue in asyncQueueReadAllNotifications().

The "pos" variable is modified within PG_TRY and then referencedwithin PG_CATCH, so for strict POSIX conformance it must be markedvolatile. Superficially the code looked safe because pos's addresswas taken, which was sufficient to force it into memory ... but it'snot sufficient to ensure that the compiler applies updates exactlywhere the program text says to. The volatility marking has to extendinto a couple of subroutines too, but I think that's probably a goodthing because the risk of out-of-order updates is mostly in thosesubroutines not asyncQueueReadAllNotifications() itself. In principlethe compiler could have re-ordered operations such that an error couldbe thrown while "pos" had an incorrect value.It's unclear how real the risk is here, but for safety back-patchto all active branches.

1 parentc70f9e8 commitc58accdCopy full SHA for c58accd

File tree

1 file changed

-5

lines changed

src/backend/commands
- async.c

1 file changed

-5

lines changed

`‎src/backend/commands/async.c`

Lines changed: 5 additions & 5 deletions

Original file line number	Diff line number	Diff line change
`@@ -369,13 +369,13 @@ static void Exec_UnlistenAllCommit(void);`
`369`	`369`	`staticboolIsListeningOn(constchar*channel);`
`370`	`370`	`staticvoidasyncQueueUnregister(void);`
`371`	`371`	`staticboolasyncQueueIsFull(void);`
`372`		`-staticboolasyncQueueAdvance(QueuePosition*position,intentryLength);`
	`372`	`+staticboolasyncQueueAdvance(volatileQueuePosition*position,intentryLength);`
`373`	`373`	`staticvoidasyncQueueNotificationToEntry(Notificationn,AsyncQueueEntryqe);`
`374`	`374`	`staticListCellasyncQueueAddEntries(ListCellnextNotify);`
`375`	`375`	`staticvoidasyncQueueFillWarning(void);`
`376`	`376`	`staticboolSignalBackends(void);`
`377`	`377`	`staticvoidasyncQueueReadAllNotifications(void);`
`378`		`-staticboolasyncQueueProcessPageEntries(QueuePosition*current,`
	`378`	`+staticboolasyncQueueProcessPageEntries(volatileQueuePosition*current,`
`379`	`379`	`QueuePositionstop,`
`380`	`380`	`char*page_buffer);`
`381`	`381`	`staticvoidasyncQueueAdvanceTail(void);`
`@@ -1202,7 +1202,7 @@ asyncQueueIsFull(void)`
`1202`	`1202`	`* returns true, else false.`
`1203`	`1203`	`*/`
`1204`	`1204`	`staticbool`
`1205`		`-asyncQueueAdvance(QueuePosition*position,intentryLength)`
	`1205`	`+asyncQueueAdvance(volatileQueuePosition*position,intentryLength)`
`1206`	`1206`	`{`
`1207`	`1207`	`intpageno=QUEUE_POS_PAGE(*position);`
`1208`	`1208`	`intoffset=QUEUE_POS_OFFSET(*position);`
`@@ -1792,7 +1792,7 @@ DisableNotifyInterrupt(void)`
`1792`	`1792`	`staticvoid`
`1793`	`1793`	`asyncQueueReadAllNotifications(void)`
`1794`	`1794`	`{`
`1795`		`-QueuePositionpos;`
	`1795`	`+volatileQueuePositionpos;`
`1796`	`1796`	`QueuePositionoldpos;`
`1797`	`1797`	`QueuePositionhead;`
`1798`	`1798`	`booladvanceTail;`
`@@ -1952,7 +1952,7 @@ asyncQueueReadAllNotifications(void)`
`1952`	`1952`	`* The QueuePosition *current is advanced past all processed messages.`
`1953`	`1953`	`*/`
`1954`	`1954`	`staticbool`
`1955`		`-asyncQueueProcessPageEntries(QueuePosition*current,`
	`1955`	`+asyncQueueProcessPageEntries(volatileQueuePosition*current,`
`1956`	`1956`	`QueuePositionstop,`
`1957`	`1957`	`char*page_buffer)`
`1958`	`1958`	`{`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commitc58accd

File tree

1 file changed

1 file changed

`‎src/backend/commands/async.c`

0 commit comments