• Tom Lane's avatar
    Don't CHECK_FOR_INTERRUPTS between WaitLatch and ResetLatch. · 887feefe
    Tom Lane authored
    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 ec9037df), 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>
    887feefe
shm_mq.c 34.8 KB