From 356d81146bafc4b4548163eb87e886c851b49e12 Mon Sep 17 00:00:00 2001 From: Bhasker Hariharan Date: Thu, 9 Jan 2020 17:56:58 -0800 Subject: Deflake a couple of TCP syscall tests when run under gotsan. PiperOrigin-RevId: 289010316 --- .../linux/socket_bind_to_device_distribution.cc | 25 ++++++++++++++++++-- test/syscalls/linux/socket_inet_loopback.cc | 27 +++++++++++++++++++--- 2 files changed, 47 insertions(+), 5 deletions(-) (limited to 'test') diff --git a/test/syscalls/linux/socket_bind_to_device_distribution.cc b/test/syscalls/linux/socket_bind_to_device_distribution.cc index 5767181a1..5ed57625c 100644 --- a/test/syscalls/linux/socket_bind_to_device_distribution.cc +++ b/test/syscalls/linux/socket_bind_to_device_distribution.cc @@ -183,7 +183,14 @@ TEST_P(BindToDeviceDistributionTest, Tcp) { } // Receive some data from a socket to be sure that the connect() // system call has been completed on another side. - int data; + // Do a short read and then close the socket to trigger a RST. This + // ensures that both ends of the connection are cleaned up and no + // goroutines hang around in TIME-WAIT. We do this so that this test + // does not timeout under gotsan runs where lots of goroutines can + // cause the test to use absurd amounts of memory. + // + // See: https://tools.ietf.org/html/rfc2525#page-50 section 2.17 + uint16_t data; EXPECT_THAT( RetryEINTR(recv)(fd.ValueOrDie().get(), &data, sizeof(data), 0), SyscallSucceedsWithValue(sizeof(data))); @@ -198,15 +205,29 @@ TEST_P(BindToDeviceDistributionTest, Tcp) { } for (int i = 0; i < kConnectAttempts; i++) { - FileDescriptor const fd = ASSERT_NO_ERRNO_AND_VALUE( + const FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE( Socket(connector.family(), SOCK_STREAM, IPPROTO_TCP)); ASSERT_THAT( RetryEINTR(connect)(fd.get(), reinterpret_cast(&conn_addr), connector.addr_len), SyscallSucceeds()); + // Do two separate sends to ensure two segments are received. This is + // required for netstack where read is incorrectly assuming a whole + // segment is read when endpoint.Read() is called which is technically + // incorrect as the syscall that invoked endpoint.Read() may only + // consume it partially. This results in a case where a close() of + // such a socket does not trigger a RST in netstack due to the + // endpoint assuming that the endpoint has no unread data. EXPECT_THAT(RetryEINTR(send)(fd.get(), &i, sizeof(i), 0), SyscallSucceedsWithValue(sizeof(i))); + + // TODO(gvisor.dev/issue/1449): Remove this block once netstack correctly + // generates a RST. + if (IsRunningOnGvisor()) { + EXPECT_THAT(RetryEINTR(send)(fd.get(), &i, sizeof(i), 0), + SyscallSucceedsWithValue(sizeof(i))); + } } // Join threads to be sure that all connections have been counted. diff --git a/test/syscalls/linux/socket_inet_loopback.cc b/test/syscalls/linux/socket_inet_loopback.cc index 619d41901..138024d9e 100644 --- a/test/syscalls/linux/socket_inet_loopback.cc +++ b/test/syscalls/linux/socket_inet_loopback.cc @@ -714,7 +714,7 @@ TEST_P(SocketInetReusePortTest, TcpPortReuseMultiThread_NoRandomSave) { sockaddr_storage listen_addr = listener.addr; sockaddr_storage conn_addr = connector.addr; constexpr int kThreadCount = 3; - constexpr int kConnectAttempts = 4096; + constexpr int kConnectAttempts = 10000; // Create the listening socket. FileDescriptor listener_fds[kThreadCount]; @@ -729,7 +729,7 @@ TEST_P(SocketInetReusePortTest, TcpPortReuseMultiThread_NoRandomSave) { ASSERT_THAT( bind(fd, reinterpret_cast(&listen_addr), listener.addr_len), SyscallSucceeds()); - ASSERT_THAT(listen(fd, kConnectAttempts / 3), SyscallSucceeds()); + ASSERT_THAT(listen(fd, 40), SyscallSucceeds()); // On the first bind we need to determine which port was bound. if (i != 0) { @@ -772,7 +772,14 @@ TEST_P(SocketInetReusePortTest, TcpPortReuseMultiThread_NoRandomSave) { } // Receive some data from a socket to be sure that the connect() // system call has been completed on another side. - int data; + // Do a short read and then close the socket to trigger a RST. This + // ensures that both ends of the connection are cleaned up and no + // goroutines hang around in TIME-WAIT. We do this so that this test + // does not timeout under gotsan runs where lots of goroutines can + // cause the test to use absurd amounts of memory. + // + // See: https://tools.ietf.org/html/rfc2525#page-50 section 2.17 + uint16_t data; EXPECT_THAT( RetryEINTR(recv)(fd.ValueOrDie().get(), &data, sizeof(data), 0), SyscallSucceedsWithValue(sizeof(data))); @@ -795,8 +802,22 @@ TEST_P(SocketInetReusePortTest, TcpPortReuseMultiThread_NoRandomSave) { connector.addr_len), SyscallSucceeds()); + // Do two separate sends to ensure two segments are received. This is + // required for netstack where read is incorrectly assuming a whole + // segment is read when endpoint.Read() is called which is technically + // incorrect as the syscall that invoked endpoint.Read() may only + // consume it partially. This results in a case where a close() of + // such a socket does not trigger a RST in netstack due to the + // endpoint assuming that the endpoint has no unread data. EXPECT_THAT(RetryEINTR(send)(fd.get(), &i, sizeof(i), 0), SyscallSucceedsWithValue(sizeof(i))); + + // TODO(gvisor.dev/issue/1449): Remove this block once netstack correctly + // generates a RST. + if (IsRunningOnGvisor()) { + EXPECT_THAT(RetryEINTR(send)(fd.get(), &i, sizeof(i), 0), + SyscallSucceedsWithValue(sizeof(i))); + } } }); -- cgit v1.2.3