Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: Andreas Seltenreich <seltenreich@gmx.de>, pgsql-hackers@postgresql.org,
Alvaro Herrera <alvherre@2ndquadrant.com>,
Michael Paquier <michael.paquier@gmail.com>
Date: 2017-10-02T21:54:20Z
Lists: pgsql-hackers
I wrote: > If this is the only problem then I'd agree we should stick to a spinlock > (I assume the strings in question can't be very long). I was thinking > more about what to do if we find other violations that are harder to fix. I took a quick look through walreceiver.c, and there are a couple of obvious problems of the same ilk in WalReceiverMain, which is doing this: walrcv->lastMsgSendTime = walrcv->lastMsgReceiptTime = walrcv->latestWalEndTime = GetCurrentTimestamp(); (ie, a potential kernel call) inside a spinlock. But there seems no real problem with just collecting the timestamp before we enter that critical section. I also don't especially like the fact that just above there it reaches elog(PANIC) with the lock still held, though at least that's a case that should never happen. Further down, it's doing a pfree() inside the spinlock, apparently for no other reason than to save one "if (tmp_conninfo)". I don't especially like the Asserts inside spinlocks, either. Personally, I think if those conditions are worth testing then they're worth testing for real (in production). Variables that are manipulated by multiple processes are way more likely to assume unexpected states than local variables. I'm also rather befuddled by the fact that this code sets and clears walrcv->latch outside the critical sections. If that field is used by any other process, surely that's completely unsafe. If it isn't, why is it being kept in shared memory? regards, tom lane
Commits
-
Fix race condition with unprotected use of a latch pointer variable.
- 2451de7e9e72 10.1 landed
- 45f9d08684d9 11.0 landed
-
Fix coding rules violations in walreceiver.c
- c9c37e335e3a 9.2.24 landed
- b24f15f86de4 9.3.20 landed
- ad40d5f74585 10.1 landed
- 89e434b59caf 11.0 landed
- 86076395ef81 9.6.6 landed
- 6b2daef96d43 9.4.15 landed
- 182abe31387c 9.5.10 landed
-
Use latch instead of select() in walreceiver
- 597a87ccc9a6 10.0 cited