Hi,
On 2022-01-30 16:51:12 +0100, Magnus Hagander wrote:
> On Sat, Jan 29, 2022 at 10:47 PM Andres Freund <and...@anarazel.de> wrote:
> >
> > Hi,
> >
> > On 2022-01-29 12:44:22 -0800, Andres Freund wrote:
> > > On 2022-01-17 10:06:56 -0800, Andres Freund wrote:
> > > > Yes, that's what I was suggesting. I wasn't thinking of using a static
> > > > var,
> > > > but putting it in StreamCtl. Note that what
> > > > pgwin32_waitforsinglesocket()
> > > > is doing doesn't protect against the problem referenced above, because
> > > > it
> > > > still is reset by WSAEventSelect.
> > >
> > > Do we are about breaking StreamCtl ABI? I don't think so?
> >
> > Here's a version of the patch only creating the event once. Needs a small
> > bit
> > of comment polishing, but otherwise I think it's sane?
>
> LGTM in general, yes.
Thanks for checking.
> I'm wondering about the part that does:
> + events[0] = stream->net_event;
> + nevents++;
> +
> + if (stream->stop_event != NULL)
> + {
> + events[1] = stream->stop_event;
> + nevents++;
> + }
> +
>
> Using a combination of nevents but hardcoded indexes does work -- but
> only as long as there is only one optional entry. Should they perhaps
> be written
> + events[nevents++] = stream->net_event;
>
> instead, for future proofing? But then you'd also have to change the
> if() statement on the return side I guess.
I did wonder about it, but the index checks get sufficiently more complicated
that it didn't quite seem worth it. It didn't seem that likely these would get
a third event to check...
I think we're going to have to generalize something like our wait events to be
frontend usable at some point. The proportion and complexity of frontend code
is increasing...
Greetings,
Andres Freund
