On Mon, May 14, 2018 at 11:49:55PM -0700, Pádraig Brady wrote:
> On 08/05/18 09:40, Daniel P. Berrangé wrote:
> > Libvirt CI recently started running "make check" on our FreeBSD 10 & 11
> > hosts, and we see frequent failure of the test-poll unit test in gnulib
> > IIUC, gnulib is not actually building a replacement poll() function
> > on FreeBSD, it is merely running the gnulib test suite against the
> > FreeBSD system impl of poll() and hitting this behaviour.
> >
> > $ ./gnulib/tests/test-poll
> > Unconnected socket test... passed
> > Connected sockets test... failed (expecting POLLHUP after shutdown)
> > General socket test with fork... failed (expecting POLLHUP after shutdown)
> > Pipe test... passed
> >
> > Looking at the first failure in test_socket_pair method.
> >
> > It sets up a listener socket, connects a client, accepts the client
> > and then closes the remote end. It expects the server's client socket
> > to thus show POLLHUP or POLLERR.
> >
> > When it fails, the poll() is in fact still showing POLLOUT. If you put
> > a sleep between the close () and poll() calls, it will succeed.
> >
> > So, IIUC, the test is racing with the BSD kernel's handling of socket
> > close - the test can't assume that just because the remote end of the
> > client has been closed, that poll() will immediately show POLLHUP|ERR.
> >
> > Anyone have ideas on how to make this test more reliable and not depend
> > on the kernel synchronizing the close() state with poll() results
> > immediately ?
> >
> > Regards,
> > Daniel
> >
>
> Yes that test looks racy as the network shutdown is async.
> How about we s/nowait/wait/, and only check for input events.
> The following works on Linux at least:
>
> --- tests/test-poll.c 2018-05-14 23:46:09.595448490 -0700
> +++ pb/gltests/test-poll.c 2018-05-14 23:45:46.827048159 -0700
> @@ -334,8 +334,9 @@
>
> test_pair (c1, c2);
> close (c1);
> - ASSERT (write (c2, "foo", 3) == 3);
> - if ((poll1_nowait (c2, POLLIN | POLLOUT) & (POLLHUP | POLLERR)) == 0)
> +
> + (void) write (c2, "foo", 3); // Initiate shutdown
> + if ((poll1_wait (c2, POLLIN) & (POLLHUP | POLLERR)) == 0)
> failed ("expecting POLLHUP after shutdown");
Unfortunately that doesn't help at all. I've tried various other tricks
to help synchronization too. For example I put sockets into blocking
mode and then did a read() which returns 0 for EOF as expected, but
poll() still returns POLLIN rather than POLLHUP after that read. I
also tried tweaking SO_LINGER to not effect. The only awful solution
I can make work is to simply sleep() a short while. eg this patch
diff --git a/tests/test-poll.c b/tests/test-poll.c
index c5e0a92de..e99ab3f4d 100644
--- a/tests/test-poll.c
+++ b/tests/test-poll.c
@@ -252,7 +252,7 @@ test_accept_first (void)
struct sockaddr_in ia;
socklen_t addrlen;
char buf[3];
- int c, pid;
+ int c, pid, i, rv;
pid = fork ();
if (pid < 0)
@@ -284,7 +284,12 @@ test_accept_first (void)
failed ("cannot read data left in the socket by closed process");
ASSERT (read (c, buf, 3) == 3);
ASSERT (write (c, "foo", 3) == 3);
- if ((poll1_wait (c, POLLIN | POLLOUT) & (POLLHUP | POLLERR)) == 0)
+ do {
+ rv = poll1_nowait (c, POLLIN | POLLOUT);
+ usleep(1000);
+ i++;
+ } while ((rv & (POLLHUP | POLLERR)) == 0 && i < 20);
+ if ((rv & (POLLHUP | POLLERR)) == 0)
failed ("expecting POLLHUP after shutdown");
close (c);
}
@@ -330,6 +335,7 @@ test_socket_pair (void)
int s = open_server_socket ();
int c1 = connect_to_socket (false);
int c2 = accept (s, (struct sockaddr *) &ia, &addrlen);
+ int i, rv;
ASSERT (s >= 0);
ASSERT (c1 >= 0);
ASSERT (c2 >= 0);
@@ -339,9 +345,14 @@ test_socket_pair (void)
test_pair (c1, c2);
close (c1);
ASSERT (write (c2, "foo", 3) == 3);
- if ((poll1_nowait (c2, POLLIN | POLLOUT) & (POLLHUP | POLLERR)) == 0)
- failed ("expecting POLLHUP after shutdown");
-
+
+ do {
+ rv = poll1_nowait (c2, POLLIN | POLLOUT);
+ usleep(1000);
+ i++;
+ } while ((rv & (POLLHUP | POLLERR)) == 0 && i < 20);
+ if ((rv & (POLLHUP | POLLERR)) == 0)
+ failed ("expecting POLLHUP after shutdown");
close (c2);
}
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
