pgsql: Don't CHECK_FOR_INTERRUPTS between WaitLatch and ResetLatch. - Mailing list pgsql-committers

From Tom Lane
Subject pgsql: Don't CHECK_FOR_INTERRUPTS between WaitLatch and ResetLatch.
Date
Msg-id E1bUIfB-0006DL-40@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Don't CHECK_FOR_INTERRUPTS between WaitLatch and ResetLatch.

This coding pattern creates a race condition, because if an interesting
interrupt happens after we've checked InterruptPending but before we reset
our latch, the latch-setting done by the signal handler would get lost,
and then we might block at WaitLatch in the next iteration without ever
noticing the interrupt condition.  You can put the CHECK_FOR_INTERRUPTS
before WaitLatch or after ResetLatch, but not between them.

Aside from fixing the bugs, add some explanatory comments to latch.h
to perhaps forestall the next person from making the same mistake.

In HEAD, also replace gather_readnext's direct call of
HandleParallelMessages with CHECK_FOR_INTERRUPTS.  It does not seem clean
or useful for this one caller to bypass ProcessInterrupts and go straight
to HandleParallelMessages; not least because that fails to consider the
InterruptPending flag, resulting in useless work both here
(if InterruptPending isn't set) and in the next CHECK_FOR_INTERRUPTS call
(if it is).

This thinko seems to have been introduced in the initial coding of
storage/ipc/shm_mq.c (commit ec9037df2), and then blindly copied into all
the subsequent parallel-query support logic.  Back-patch relevant hunks
to 9.4 to extirpate the error everywhere.

Discussion: <1661.1469996911@sss.pgh.pa.us>

Branch
------
REL9_5_STABLE

Details
-------
http://git.postgresql.org/pg/commitdiff/8ef3d9fae496d80fc1d100d49b46891ae9c2c0fc

Modified Files
--------------
src/backend/libpq/pqmq.c             |  2 +-
src/backend/storage/ipc/shm_mq.c     | 18 +++++++++---------
src/include/storage/latch.h          | 16 ++++++++++++++++
src/test/modules/test_shm_mq/setup.c |  6 +++---
src/test/modules/test_shm_mq/test.c  |  2 +-
5 files changed, 30 insertions(+), 14 deletions(-)


pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: pgsql: Don't CHECK_FOR_INTERRUPTS between WaitLatch and ResetLatch.
Next
From: Tom Lane
Date:
Subject: pgsql: Don't CHECK_FOR_INTERRUPTS between WaitLatch and ResetLatch.