Re: Hang in pldebugger after git commit : 98a64d0

On Fri, Dec 16, 2016 at 9:03 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Dec 15, 2016 at 4:17 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Thu, Dec 15, 2016 at 10:04 AM, Andres Freund <andres(at)anarazel(dot)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().
>

makes sense.

> I think it should be the responsibility of
> WaitEventSetWaitBlock() to reset the event, if needed, before calling
> WaitForMultipleObjects().
>

If we want to change WaitEventSetWaitBlock then ideally we need to
change all other wait API's (WAIT_USE_SELECT, WAIT_USE_POLL, etc.) as
well. I don't see the need for it, can't we do it in
WaitEventSetWait() after processing all the returned events. It can
be done by enumerating all wait events and reset the events for which
reset flag is true. If we see code prior to 9.6, this event (socket
event) has been reset only after processing all returned events, not
after every event.

> 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
>

Isn't the statement in above doc. "For FD_READ, FD_OOB, and FD_ACCEPT
network events, network event recording and event object signaling are
level-triggered." says that FD_READ events are level-triggered which
seems to be contradictory with above comment?

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

Thanks for the information.

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