Re: Hang in pldebugger after git commit : 98a64d0

On Wed, Dec 14, 2016 at 5:35 AM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>> I have just read the patch, and hardcoding the array position for a
>> socket event to 0 assumes that the socket event will be *always* the
>> first one in the list. but that cannot be true all the time, any code
>> that does not register the socket event as the first one in the list
>> will likely break.
>
> I think your point is very valid and even i had similar thought in my
> mind when implementing it but as i mentioned upthread that's how it is
> being used in the current code base. Please check a function call to
> ModifyWaitEvent() in secure_read().

That's kind of irrelevant. A WaitEventSet is a general-purpose
primitive, and it needs to work in all situations, current and future,
where a reasonable developer would expect it to work. Sure, pq_init()
puts the socket in FeBeWaitSet at position 0. However, in
WaitLatchOrSocket, the socket event, if required, is added after the
latch and postmaster-death events. And new code written using
CreateWaitEventSet() / AddWaitEventToSet() in the future could put a
socket at any offset in the event array, or could add multiple
sockets. So Michael is right: you can't just stick a hack into
WaitEventSetWait that assumes the socket is at offset 0 even if that
happens to fix the problem we are facing right at this moment.

(I am not sure whether Michael's proposed placement is correct, but
I'm almost 100% sure the placement in the submitted patch is not.)

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