Hi, On Thu, Mar 21, 2019 at 12:33:58AM +0100, Steinar H. Gunderson wrote: > On Sat, Mar 09, 2019 at 07:26:52PM +0100, Santiago Vila wrote: > > Whoever wants to reproduce this (and possibly debug it), *please* > > contact me privately and I will gladly provide ssh access to a machine > > where it happens very often. > > I've looked briefly into this. > > First, to reproduce this reliably, simply restrict it to one core > (taskset -c 0 /build/gtk3/src/vncconnectiontest). The reason will be fairly > clear from the text below. > > As far as I can see, this test simulates a broken VNC server (to test the > client's robustness). It says it's got a 100x100 true color display, but then > goes on and starts sending a color map. > > As soon as the client receives the information about the color map, > it realizes something is wrong, and a race starts. Now the client wants to > close the connection at the same time as the fake server wants to keep > sending the cmap data. If you've got two cores, the server just keeps on > sending data happily; by the time it's noticed that the client is gone, > the test has passed and all is fine. But if you've only got one, then the > first byte of the cmap causes a context switch over to the client, which then > gets ample time to read the data and close the socket before the server gets > to send the next byte. The server thus gets EPIPE, and test_send_u16() > breaks. > > I believe the right fix is to send every byte after the first “set color map” > byte using a non-asserting send. When we've done something invalid, we'd > better be ready for sending data to fail. > > Please try the attached diff; it fixes the problem for me. I can NMU if the > maintainers want.
I wouldn't mind an NMU. -- Guido > > /* Steinar */ > -- > Homepage: https://www.sesse.net/ > Index: gtk-vnc-0.9.0/src/vncconnectiontest.c > =================================================================== > --- gtk-vnc-0.9.0.orig/src/vncconnectiontest.c > +++ gtk-vnc-0.9.0/src/vncconnectiontest.c > @@ -56,12 +56,23 @@ static void test_send_u8(GOutputStream * > g_assert(g_output_stream_write_all(os, &v, 1, NULL, NULL, NULL)); > } > > +static void send_u8(GOutputStream *os, guint8 v) > +{ > + g_output_stream_write_all(os, &v, 1, NULL, NULL, NULL); > +} > + > static void test_send_u16(GOutputStream *os, guint16 v) > { > v = GUINT16_TO_BE(v); > g_assert(g_output_stream_write_all(os, &v, 2, NULL, NULL, NULL)); > } > > +static void send_u16(GOutputStream *os, guint16 v) > +{ > + v = GUINT16_TO_BE(v); > + g_output_stream_write_all(os, &v, 2, NULL, NULL, NULL); > +} > + > static void test_send_u32(GOutputStream *os, guint32 v) > { > v = GUINT32_TO_BE(v); > @@ -429,18 +440,18 @@ static void test_unexpected_cmap_server( > test_recv_u16(is, 100); > test_recv_u16(is, 100); > > - /* set color map */ > + /* set color map -- after this, the client may close the connection at > any time */ > test_send_u8(os, 1); > /* pad */ > - test_send_u8(os, 0); > + send_u8(os, 0); > /* first color, ncolors */ > - test_send_u16(os, 0); > - test_send_u16(os, 1); > + send_u16(os, 0); > + send_u16(os, 1); > > /* r,g,b */ > - test_send_u16(os, 128); > - test_send_u16(os, 128); > - test_send_u16(os, 128); > + send_u16(os, 128); > + send_u16(os, 128); > + send_u16(os, 128); > } > > > @@ -505,11 +516,13 @@ static void test_overflow_cmap_server(GI > test_send_u16(os, 65535); > test_send_u16(os, 2); > > + /* after this, the client may close the connection at any time */ > + > /* r,g,b */ > for (int i = 0 ; i < 2; i++) { > - test_send_u16(os, i); > - test_send_u16(os, i); > - test_send_u16(os, i); > + send_u16(os, i); > + send_u16(os, i); > + send_u16(os, i); > } > } > > _______________________________________________ > Pkg-libvirt-maintainers mailing list > pkg-libvirt-maintain...@alioth-lists.debian.net > https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-libvirt-maintainers
