On Wed, Sep 7, 2022 at 1:07 PM Bin Meng <bmeng...@gmail.com> wrote: > > Hi Clément, > > On Tue, Sep 6, 2022 at 8:06 PM Clément Chigot <chi...@adacore.com> wrote: > > > > > > > I checked your patch, what you did seems to be something one would > > > > > naturally write, but what is currently in the QEMU sources seems to be > > > > > written intentionally. > > > > > > > > > > +Paolo Bonzini , you are the one who implemented the socket watch on > > > > > Windows. Could you please help analyze this issue? > > > > > > > > > > > to avoid WSAEnumNetworkEvents for the master GSource which only has > > > > > > G_IO_HUP (or for any GSource having only that). > > > > > > As I said above, the current code doesn't do anything with it > > > > > > anyway. > > > > > > So, IMO, it's safe to do so. > > > > > > > > > > > > I'll send you my patch attached. I was planning to send it in the > > > > > > following > > > > > > weeks anyway. I was just waiting to be sure everything looks fine > > > > > > on our > > > > > > CI. Feel free to test and modify it if needed. > > > > > > > > > > I tested your patch. Unfortunately there is still one test case > > > > > (migration-test.exe) throwing up the "Broken pipe" message. > > > > > > > > I must say I didn't fully test it against qemu testsuite yet. Maybe > > > > there are > > > > some refinements to be done. "Broken pipe" might be linked to the > > > > missing > > > > G_IO_HUP support. > > > > > > > > > Can you test my patch instead to see if your gdb issue can be fixed? > > > > > > > > Yeah sure. I'll try to do it this afternoon. > > > > I can't explain how mad at me I am... I'm pretty sure your patch was the > > first > > thing I've tried when I encountered this issue. But it wasn't working > > or IIRC the > > issue went away but that was because the polling was actually disabled > > (looping > > indefinitely)...I'm suspecting that I already had changed the CreateEvent > > for > > WSACreateEvent which forces you to handle the reset. > > Finally, I end up struggling reworking the whole check function... > > But yeah, your patch does work fine on my gdb issues too. > > Good to know this patch works for you too. > > > And I guess the events are reset when recv() is being called because of the > > auto-reset feature set up by CreateEvent(). > > IIUC, what Marc-André means by busy loop is the polling being looping > > indefinitely as I encountered. I can ensure that this patch doesn't do that. > > It can be easily checked by setting the env variable G_MAIN_POLL_DEBUG. > > It'll show what g_poll is doing and it's normally always available on > > Windows. > > > > Anyway, we'll wait for Paolo to see if he remembers why he had to call > > WSAEnumNetworkEvents. Otherwize, let's go for your patch. Mine might > > be a good start to improve the whole polling on Windows but if it doesn't > > work in your case, it then needs some refinements. > > > > Yeah, this issue bugged me quite a lot. If we want to reset the event > in qio_channel_socket_source_check(), we will have to do the following > to make sure qtests are happy. > > diff --git a/io/channel-watch.c b/io/channel-watch.c > index 43d38494f7..f1e1650b81 100644 > --- a/io/channel-watch.c > +++ b/io/channel-watch.c > @@ -124,8 +124,6 @@ qio_channel_socket_source_check(GSource *source) > return 0; > } > - WSAEnumNetworkEvents(ssource->socket, ssource->ioc->event, &ev); > - > FD_ZERO(&rfds); > FD_ZERO(&wfds); > FD_ZERO(&xfds); > @@ -153,6 +151,10 @@ qio_channel_socket_source_check(GSource *source) > ssource->revents |= G_IO_PRI; > } > + if (ssource->revents) { > + WSAEnumNetworkEvents(ssource->socket, ssource->ioc->event, &ev); > + } > + > return ssource->revents; > } > > Removing "if (ssource->revents)" won't work. > > It seems to me that resetting the event twice (one time with the > master Gsource, and the other time with the child GSource) causes some > bizarre behavior. But MSDN [1] says > > "Resetting an event that is already reset has no effect." > > [1] > https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-resetevent >
Paolo, any comments about this issue? Regards, Bin