Hi,
On 2022-01-30 16:51:12 +0100, Magnus Hagander wrote:
> On Sat, Jan 29, 2022 at 10:47 PM Andres Freund <andres@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