From: Adam Simpkins <simpkins@fb.com>
Date: Tue, 5 Jul 2016 18:20:28 +0000 (-0700)
Subject: fix flaky ConnectTFOTimeout and ConnectTFOFallbackTimeout tests
X-Git-Tag: 2016.07.26~79
X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=a3bd593ad9374cd3a1db31066874b9bad1cf74b4

fix flaky ConnectTFOTimeout and ConnectTFOFallbackTimeout tests

Summary:
In the ConnectTFOTimeout and ConnectTFOFallbackTimeout tests in
AsyncSSLSocketTest.cpp, the client runs for 1ms before timing out and quitting.
It may end up shutting down the server thread before the server has even
received the TCP connect callback.  If this happened it would cause the test to
fail, since the server code checked to make sure the callback was invoked.

This diff creates a new ConnectTimeoutCallback server-side callback for these
tests to use, which doesn't care if it gets told about a new connection or not.

Reviewed By: siyengar

Differential Revision: D3512809

fbshipit-source-id: ce77fe944fb06a38a84c1458356f161cec7387b3
---

diff --git a/folly/io/async/test/AsyncSSLSocketTest.cpp b/folly/io/async/test/AsyncSSLSocketTest.cpp
index 27805c7b..cdacacad 100644
--- a/folly/io/async/test/AsyncSSLSocketTest.cpp
+++ b/folly/io/async/test/AsyncSSLSocketTest.cpp
@@ -1794,9 +1794,11 @@ class ConnCallback : public AsyncSocket::ConnectCallback {
   State state{State::WAITING};
 };
 
+template <class Cardinality>
 MockAsyncTFOSSLSocket::UniquePtr setupSocketWithFallback(
     EventBase* evb,
-    const SocketAddress& address) {
+    const SocketAddress& address,
+    Cardinality cardinality) {
   // Set up SSL context.
   auto sslContext = std::make_shared<SSLContext>();
 
@@ -1806,6 +1808,7 @@ MockAsyncTFOSSLSocket::UniquePtr setupSocketWithFallback(
   socket->enableTFO();
 
   EXPECT_CALL(*socket, tfoSendMsg(_, _, _))
+      .Times(cardinality)
       .WillOnce(Invoke([&](int fd, struct msghdr*, int) {
         sockaddr_storage addr;
         auto len = address.getAddress(&addr);
@@ -1824,7 +1827,7 @@ TEST(AsyncSSLSocketTest, ConnectWriteReadCloseTFOFallback) {
 
   EventBase evb;
 
-  auto socket = setupSocketWithFallback(&evb, server.getAddress());
+  auto socket = setupSocketWithFallback(&evb, server.getAddress(), 1);
   ConnCallback ccb;
   socket->connect(&ccb, server.getAddress(), 30);
 
@@ -1852,10 +1855,7 @@ TEST(AsyncSSLSocketTest, ConnectWriteReadCloseTFOFallback) {
 
 TEST(AsyncSSLSocketTest, ConnectTFOTimeout) {
   // Start listening on a local port
-  WriteCallbackBase writeCallback;
-  ReadErrorCallback readCallback(&writeCallback);
-  HandshakeCallback handshakeCallback(&readCallback);
-  SSLServerAcceptCallback acceptCallback(&handshakeCallback);
+  ConnectTimeoutCallback acceptCallback;
   TestSSLServer server(&acceptCallback, true);
 
   // Set up SSL context.
@@ -1871,15 +1871,12 @@ TEST(AsyncSSLSocketTest, ConnectTFOTimeout) {
 
 TEST(AsyncSSLSocketTest, ConnectTFOFallbackTimeout) {
   // Start listening on a local port
-  WriteCallbackBase writeCallback;
-  ReadErrorCallback readCallback(&writeCallback);
-  HandshakeCallback handshakeCallback(&readCallback);
-  SSLServerAcceptCallback acceptCallback(&handshakeCallback);
+  ConnectTimeoutCallback acceptCallback;
   TestSSLServer server(&acceptCallback, true);
 
   EventBase evb;
 
-  auto socket = setupSocketWithFallback(&evb, server.getAddress());
+  auto socket = setupSocketWithFallback(&evb, server.getAddress(), AtMost(1));
   ConnCallback ccb;
   // Set a short timeout
   socket->connect(&ccb, server.getAddress(), 1);
diff --git a/folly/io/async/test/AsyncSSLSocketTest.h b/folly/io/async/test/AsyncSSLSocketTest.h
index e4b51249..42bb03ac 100644
--- a/folly/io/async/test/AsyncSSLSocketTest.h
+++ b/folly/io/async/test/AsyncSSLSocketTest.h
@@ -97,7 +97,7 @@ public AsyncTransportWrapper::ReadCallback {
       : wcb_(wcb), state(STATE_WAITING) {}
 
   ~ReadCallbackBase() {
-    EXPECT_EQ(state, STATE_SUCCEEDED);
+    EXPECT_EQ(STATE_SUCCEEDED, state);
   }
 
   void setSocket(
@@ -352,7 +352,7 @@ public:
   }
 
   ~HandshakeCallback() {
-    EXPECT_EQ(state, STATE_SUCCEEDED);
+    EXPECT_EQ(STATE_SUCCEEDED, state);
   }
 
   void closeSocket() {
@@ -380,7 +380,7 @@ public:
   state(STATE_WAITING), hcb_(hcb) {}
 
   ~SSLServerAcceptCallbackBase() {
-    EXPECT_EQ(state, STATE_SUCCEEDED);
+    EXPECT_EQ(STATE_SUCCEEDED, state);
   }
 
   void acceptError(const std::exception& ex) noexcept override {
@@ -587,6 +587,25 @@ public:
   }
 };
 
+class ConnectTimeoutCallback : public SSLServerAcceptCallbackBase {
+ public:
+  ConnectTimeoutCallback() : SSLServerAcceptCallbackBase(nullptr) {
+    // We don't care if we get invoked or not.
+    // The client may time out and give up before connAccepted() is even
+    // called.
+    state = STATE_SUCCEEDED;
+  }
+
+  // Functions inherited from TAsyncSSLServerSocket::SSLAcceptCallback
+  void connAccepted(
+      const std::shared_ptr<folly::AsyncSSLSocket>& s) noexcept override {
+    std::cerr << "ConnectTimeoutCallback::connAccepted" << std::endl;
+
+    // Just wait a while before closing the socket, so the client
+    // will time out waiting for the handshake to complete.
+    s->getEventBase()->tryRunAfterDelay([=] { s->close(); }, 100);
+  }
+};
 
 class TestSSLServer {
  protected: