Re: [HACKERS] Hang in pldebugger after git commit : 98a64d0

On Thu, Dec 15, 2016 at 4:17 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Thu, Dec 15, 2016 at 10:04 AM, Andres Freund <andres@anarazel.de> wrote:
>> Hi,
>>
>> On 2016-12-12 16:46:38 +0900, Michael Paquier wrote:
>>> Ashutosh, could you try it and see if it improves things?
>>
>> So what's the theory of why this triggers pldebugger hangs, but
>> apparently causes not many other problems?
>>
>
> The theory is that with pldebugger latch event gets triggered before
> FD_READ due to which it seems like the second event gets lost and
> WaitForMultipleObjects() keeps on waiting indefinitely.  The probable
> reason is that we fail to reset the event due to which we are seeing
> this behavior. That seems to be required as per msdn as well, as
> pointed by Robert upthread.
>
> Find attached patch to implement the resetting of event only for the
> required case.  This fixes the reported problem.

After some study I don't think this is quite right yet.  The client
code is not obliged to call ModifyWaitEvent() before again calling
WaitEventSetWait().  I think it should be the responsibility of
WaitEventSetWaitBlock() to reset the event, if needed, before calling
WaitForMultipleObjects().  BTW, I suggest this rewritten comment:
           /*------            * FD_READ events are edge-triggered on Windows per            *
https://msdn.microsoft.com/en-us/library/windows/desktop/ms741576(v=vs.85).aspx           * but users of the latch
facilityare entitled to expect            * level-triggered behavior.  Indeed, this function itself            *
expectslevel-triggered behavior, since it doesn't guarantee            * that a read event will be returned if the
latchis set at the            * same time.  Even if it did, the caller might drop that event            * expecting it
toreoccur on next call.  So, we must force            * the event to be reset if this WaitEventSet is used again in
      * order to avoid an indefinite hang.            *------            */
 

The dashes keep pgindent from inserting a line break into the link.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company