fix flaky ConnectTFOTimeout and ConnectTFOFallbackTimeout tests
authorAdam Simpkins <simpkins@fb.com>
Tue, 5 Jul 2016 18:20:28 +0000 (11:20 -0700)
committerFacebook Github Bot 7 <facebook-github-bot-7-bot@fb.com>
Tue, 5 Jul 2016 18:23:29 +0000 (11:23 -0700)
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

folly/io/async/test/AsyncSSLSocketTest.cpp
folly/io/async/test/AsyncSSLSocketTest.h

index 27805c7b3cfb8d8c717916a4fa11d11bad81fcdb..cdacacada0a75d22c407c03b005446edcf351c63 100644 (file)
@@ -1794,9 +1794,11 @@ class ConnCallback : public AsyncSocket::ConnectCallback {
   State state{State::WAITING};
 };
 
   State state{State::WAITING};
 };
 
+template <class Cardinality>
 MockAsyncTFOSSLSocket::UniquePtr setupSocketWithFallback(
     EventBase* evb,
 MockAsyncTFOSSLSocket::UniquePtr setupSocketWithFallback(
     EventBase* evb,
-    const SocketAddress& address) {
+    const SocketAddress& address,
+    Cardinality cardinality) {
   // Set up SSL context.
   auto sslContext = std::make_shared<SSLContext>();
 
   // Set up SSL context.
   auto sslContext = std::make_shared<SSLContext>();
 
@@ -1806,6 +1808,7 @@ MockAsyncTFOSSLSocket::UniquePtr setupSocketWithFallback(
   socket->enableTFO();
 
   EXPECT_CALL(*socket, tfoSendMsg(_, _, _))
   socket->enableTFO();
 
   EXPECT_CALL(*socket, tfoSendMsg(_, _, _))
+      .Times(cardinality)
       .WillOnce(Invoke([&](int fd, struct msghdr*, int) {
         sockaddr_storage addr;
         auto len = address.getAddress(&addr);
       .WillOnce(Invoke([&](int fd, struct msghdr*, int) {
         sockaddr_storage addr;
         auto len = address.getAddress(&addr);
@@ -1824,7 +1827,7 @@ TEST(AsyncSSLSocketTest, ConnectWriteReadCloseTFOFallback) {
 
   EventBase evb;
 
 
   EventBase evb;
 
-  auto socket = setupSocketWithFallback(&evb, server.getAddress());
+  auto socket = setupSocketWithFallback(&evb, server.getAddress(), 1);
   ConnCallback ccb;
   socket->connect(&ccb, server.getAddress(), 30);
 
   ConnCallback ccb;
   socket->connect(&ccb, server.getAddress(), 30);
 
@@ -1852,10 +1855,7 @@ TEST(AsyncSSLSocketTest, ConnectWriteReadCloseTFOFallback) {
 
 TEST(AsyncSSLSocketTest, ConnectTFOTimeout) {
   // Start listening on a local port
 
 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.
   TestSSLServer server(&acceptCallback, true);
 
   // Set up SSL context.
@@ -1871,15 +1871,12 @@ TEST(AsyncSSLSocketTest, ConnectTFOTimeout) {
 
 TEST(AsyncSSLSocketTest, ConnectTFOFallbackTimeout) {
   // Start listening on a local port
 
 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;
 
   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);
   ConnCallback ccb;
   // Set a short timeout
   socket->connect(&ccb, server.getAddress(), 1);
index e4b512496c3941afbe5ea2f2cae69597c1daa46d..42bb03ac135a7f7dfe71fda0be60a7884e02f6c0 100644 (file)
@@ -97,7 +97,7 @@ public AsyncTransportWrapper::ReadCallback {
       : wcb_(wcb), state(STATE_WAITING) {}
 
   ~ReadCallbackBase() {
       : wcb_(wcb), state(STATE_WAITING) {}
 
   ~ReadCallbackBase() {
-    EXPECT_EQ(state, STATE_SUCCEEDED);
+    EXPECT_EQ(STATE_SUCCEEDED, state);
   }
 
   void setSocket(
   }
 
   void setSocket(
@@ -352,7 +352,7 @@ public:
   }
 
   ~HandshakeCallback() {
   }
 
   ~HandshakeCallback() {
-    EXPECT_EQ(state, STATE_SUCCEEDED);
+    EXPECT_EQ(STATE_SUCCEEDED, state);
   }
 
   void closeSocket() {
   }
 
   void closeSocket() {
@@ -380,7 +380,7 @@ public:
   state(STATE_WAITING), hcb_(hcb) {}
 
   ~SSLServerAcceptCallbackBase() {
   state(STATE_WAITING), hcb_(hcb) {}
 
   ~SSLServerAcceptCallbackBase() {
-    EXPECT_EQ(state, STATE_SUCCEEDED);
+    EXPECT_EQ(STATE_SUCCEEDED, state);
   }
 
   void acceptError(const std::exception& ex) noexcept override {
   }
 
   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:
 
 class TestSSLServer {
  protected: