Hi
On Wed, Aug 24, 2022 at 2:49 PM Bin Meng <bmeng...@gmail.com> wrote:
> From: Bin Meng <bin.m...@windriver.com>
>
> Random failure was observed when running qtests on Windows due to
> "Broken pipe" detected by qmp_fd_receive(). What happened is that
> the qtest executable sends testing data over a socket to the QEMU
> under test but no response is received. The errno of the recv()
> call from the qtest executable indicates ETIMEOUT, due to the qmp
> chardev's tcp_chr_read() is never called to receive testing data
> hence no response is sent to the other side.
>
> tcp_chr_read() is registered as the callback of the socket watch
> GSource. The reason of the callback not being called by glib, is
> that the source check fails to indicate the source is ready. There
> are two socket watch sources created to monitor the same socket
> event object from the char-socket backend in update_ioc_handlers().
>
During the source check phase, qio_channel_socket_source_check()
> calls WSAEnumNetworkEvents() to discovers occurrences of network
> events for the indicated socket, clear internal network event records,
> and reset the event object. Testing shows that if we don't reset the
> event object by not passing the event handle to WSAEnumNetworkEvents()
> the symptom goes away and qtest runs very stably.
>
> It looks we don't need to call WSAEnumNetworkEvents() at all, as we
> don't parse the result of WSANETWORKEVENTS returned from this API.
> We use select() to poll the socket status. Fix this instability by
> dropping the WSAEnumNetworkEvents() call.
>
> Signed-off-by: Bin Meng <bin.m...@windriver.com>
>
What clears the event then?
> ---
> During the testing, I removed the following codes in update_ioc_handlers():
>
> remove_hup_source(s);
> s->hup_source = qio_channel_create_watch(s->ioc, G_IO_HUP);
> g_source_set_callback(s->hup_source, (GSourceFunc)tcp_chr_hup,
> chr, NULL);
> g_source_attach(s->hup_source, chr->gcontext);
>
> and such change also makes the symptom go away.
>
> And if I moved the above codes to the beginning, before the call to
> io_add_watch_poll(), the symptom also goes away.
>
> It seems two sources watching on the same socket event object is
> the key that leads to the instability. The order of adding a source
> watch seems to also play a role but I can't explain why.
> Hopefully a Windows and glib expert could explain this behavior.
>
>
Feel free to leave that comment in the commit message.
This is strange, as both sources should have different events, clearing one
shouldn't affect the other.
I guess it's WSAEnumNetworkEvents clearing of the internal network event
records that is problematic.
Can you check if you replace the call with ResetEvent() everything works?
> io/channel-watch.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/io/channel-watch.c b/io/channel-watch.c
> index 89f3c8a88a..e34d86e810 100644
> --- a/io/channel-watch.c
> +++ b/io/channel-watch.c
> @@ -115,17 +115,13 @@ static gboolean
> qio_channel_socket_source_check(GSource *source)
> {
> static struct timeval tv0;
> -
> QIOChannelSocketSource *ssource = (QIOChannelSocketSource *)source;
> - WSANETWORKEVENTS ev;
> fd_set rfds, wfds, xfds;
>
> if (!ssource->condition) {
> return 0;
> }
>
> - WSAEnumNetworkEvents(ssource->socket, ssource->ioc->event, &ev);
> -
> FD_ZERO(&rfds);
> FD_ZERO(&wfds);
> FD_ZERO(&xfds);
>
Unrelated, after this chunk, there is
FD_SET((SOCKET)ssource->socket, &rfds);
it seems we can drop the cast. Feel free to send another patch.
--
Marc-André Lureau
