On Mon, Aug 1, 2016 at 8:45 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Amit Kapila <amit.kapil...@gmail.com> writes:
>> On Mon, Aug 1, 2016 at 1:58 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
>>> I believe this is wrong and the CHECK_FOR_INTERRUPTS needs to be before
>>> or after the two latch operations.  As-is, if the reason somebody set
>>> our latch was to get us to notice that a CHECK_FOR_INTERRUPTS condition
>>> happened, there's a race condition where we'd fail to realize that.
>> I could see that in nodeGather.c, it might fail to notice the SetLatch
>> done by worker process or spuriously woken up due to SetLatch for some
>> unrelated reason.  However, I don't see what problem it can cause
>> apart from one extra loop cycle where it will try to process the tuple
>> when actually there is no tuple in the queue.
> Consider the following sequence of events:
> 1. gather_readnext reaches the WaitLatch, and is allowed to pass through
> it for some unrelated reason, perhaps some long-since-handled SIGUSR1
> from a worker process.
> 2. gather_readnext does CHECK_FOR_INTERRUPTS(), and sees nothing pending.
> 3. A SIGINT is received.  StatementCancelHandler sets QueryCancelPending
> and does SetLatch(MyLatch).
> 4. gather_readnext does ResetLatch(MyLatch).
> 5. gather_readnext runs through its loop again, finds nothing to do, and
> reaches the WaitLatch.
> 6. The process is now sleeping on its latch, and might sit there a long
> time before noticing the pending query cancel.
> Obviously the window for this race condition is pretty tight --- there's
> not many instructions between steps 2 and 4.  But it can happen.  If
> memory serves, we've had actual field reports for race condition bugs
> where the window that was being hit wasn't much more than a single
> instruction.
> Also, it's entirely possible that the bug could be masked, if there was
> another CHECK_FOR_INTERRUPTS lurking anywhere in the code called within
> the loop.  That doesn't excuse this coding practice, though.

Right and Thanks for the detailed explanation.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to