add fdAttach callback

Summary: The fdAttach callback notifies all AsyncSocketLifecycleObservers attached to the socket that a file descriptor has just been associated with the socket. In AsyncSocket, the file descriptor is created when connect() is **called**, not when it succeeds. This callback lets observers know about the fd immediately; otherwise, observers would have to wait until the connect succeeds to learn about the file descriptor attached event.

Reviewed By: bschlinker

Differential Revision: D29910574

fbshipit-source-id: c452f1091c657a7d118b1e06546555aa04a34bf1

folly/io/async/AsyncSocket.cpp

@@ -763,12 +763,9 @@ void AsyncSocket::connect(
           withAddr("failed to create socket"),
           errnoCopy);
     }
-    disableTransparentFunctions(fd_, noTransparentTls_, noTSocks_);
-    if (const auto shutdownSocketSet = wShutdownSocketSet_.lock()) {
-      shutdownSocketSet->add(fd_);
-    }
-    ioHandler_.changeHandlerFD(fd_);
 
+    disableTransparentFunctions(fd_, noTransparentTls_, noTSocks_);
+    handleNetworkSocketAttached();
     setCloseOnExec();
 
     // Put the socket in non-blocking mode
@@ -3061,6 +3058,22 @@ void AsyncSocket::timeoutExpired() noexcept {
   }
 }
 
+void AsyncSocket::handleNetworkSocketAttached() {
+  VLOG(6) << "AsyncSocket::attachFd(this=" << this << ", fd=" << fd_
+          << ", evb=" << eventBase_ << " , state=" << state_
+          << ", events=" << std::hex << eventFlags_ << ")";
+  for (const auto& cb : lifecycleObservers_) {
+    if (auto dCb = dynamic_cast<AsyncSocket::LifecycleObserver*>(cb)) {
+      dCb->fdAttach(this);
+    }
+  }
+
+  if (const auto shutdownSocketSet = wShutdownSocketSet_.lock()) {
+    shutdownSocketSet->add(fd_);
+  }
+  ioHandler_.changeHandlerFD(fd_);
+}
+
 ssize_t AsyncSocket::tfoSendMsg(
     NetworkSocket fd, struct msghdr* msg, int msg_flags) {
   return detail::tfo_sendmsg(fd, msg, msg_flags);

folly/io/async/AsyncSocket.h

@@ -1109,6 +1109,14 @@ class AsyncSocket : public AsyncTransport {
      */
     virtual void fdDetach(AsyncSocket* /* socket */) noexcept = 0;
 
+    /**
+     * fdAttach() is invoked when the socket file descriptor is attached.
+     *
+     * @param socket      Socket for which handleNetworkSocketAttached was
+     * invoked.
+     */
+    virtual void fdAttach(AsyncSocket* /* socket */) noexcept {}
+
     /**
      * move() will be invoked when a new AsyncSocket is being constructed via
      * constructor AsyncSocket(AsyncSocket* oldAsyncSocket) from an AsyncSocket
@@ -1311,6 +1319,16 @@ class AsyncSocket : public AsyncTransport {
   virtual void handleConnect() noexcept;
   void timeoutExpired() noexcept;
 
+  /**
+   * Handler for when the file descriptor is attached to the AsyncSocket.
+
+   * This updates the EventHandler to start using the fd and notifies all
+   * observers attached to the socket. This is necessary to let
+   * observers know about an attached fd immediately (i.e., on connection
+   * attempt) rather than when the connection succeeds.
+   */
+  virtual void handleNetworkSocketAttached();
+
   /**
    * Attempt to read from the socket into a single buffer
    *

folly/io/async/test/AsyncSocketTest2.cpp

@@ -7576,6 +7576,7 @@ TEST(AsyncSocket, LifecycleObserverAttachThenDestroySocket) {
   Mock::VerifyAndClearExpectations(cb.get());
 
   EXPECT_CALL(*cb, connectMock(socket.get()));
+  EXPECT_CALL(*cb, fdAttachMock(socket.get()));
   socket->connect(nullptr, server.getAddress(), 30);
   evb.loop();
   Mock::VerifyAndClearExpectations(cb.get());
@@ -7610,6 +7611,8 @@ TEST(AsyncSocket, LifecycleObserverMultipleAttachThenDestroySocket) {
 
   EXPECT_CALL(*cb1, connectMock(socket.get()));
   EXPECT_CALL(*cb2, connectMock(socket.get()));
+  EXPECT_CALL(*cb1, fdAttachMock(socket.get()));
+  EXPECT_CALL(*cb2, fdAttachMock(socket.get()));
   socket->connect(nullptr, server.getAddress(), 30);
   evb.loop();
   Mock::VerifyAndClearExpectations(cb1.get());
@@ -7752,6 +7755,7 @@ TEST(AsyncSocket, LifecycleObserverDetach) {
   Mock::VerifyAndClearExpectations(cb.get());
 
   EXPECT_CALL(*cb, connectMock(socket1.get()));
+  EXPECT_CALL(*cb, fdAttachMock(socket1.get()));
   socket1->connect(nullptr, server.getAddress(), 30);
   evb.loop();
   Mock::VerifyAndClearExpectations(cb.get());
@@ -7780,6 +7784,7 @@ TEST(AsyncSocket, LifecycleObserverMoveResubscribe) {
   Mock::VerifyAndClearExpectations(cb.get());
 
   EXPECT_CALL(*cb, connectMock(socket1.get()));
+  EXPECT_CALL(*cb, fdAttachMock(socket1.get()));
   socket1->connect(nullptr, server.getAddress(), 30);
   evb.loop();
   Mock::VerifyAndClearExpectations(cb.get());
@@ -7826,6 +7831,7 @@ TEST(AsyncSocket, LifecycleObserverMoveDoNotResubscribe) {
   Mock::VerifyAndClearExpectations(cb.get());
 
   EXPECT_CALL(*cb, connectMock(socket1.get()));
+  EXPECT_CALL(*cb, fdAttachMock(socket1.get()));
   socket1->connect(nullptr, server.getAddress(), 30);
   evb.loop();
   Mock::VerifyAndClearExpectations(cb.get());
@@ -7877,6 +7883,7 @@ TEST(AsyncSocket, LifecycleObserverDetachCallbackAfterConnect) {
   Mock::VerifyAndClearExpectations(cb.get());
 
   EXPECT_CALL(*cb, connectMock(socket.get()));
+  EXPECT_CALL(*cb, fdAttachMock(socket.get()));
   socket->connect(nullptr, server.getAddress(), 30);
   evb.loop();
   Mock::VerifyAndClearExpectations(cb.get());
@@ -7897,6 +7904,7 @@ TEST(AsyncSocket, LifecycleObserverDetachCallbackAfterClose) {
   Mock::VerifyAndClearExpectations(cb.get());
 
   EXPECT_CALL(*cb, connectMock(socket.get()));
+  EXPECT_CALL(*cb, fdAttachMock(socket.get()));
   socket->connect(nullptr, server.getAddress(), 30);
   evb.loop();
   Mock::VerifyAndClearExpectations(cb.get());
@@ -7921,6 +7929,7 @@ TEST(AsyncSocket, LifecycleObserverDetachCallbackcloseDuringDestroy) {
   Mock::VerifyAndClearExpectations(cb.get());
 
   EXPECT_CALL(*cb, connectMock(socket.get()));
+  EXPECT_CALL(*cb, fdAttachMock(socket.get()));
   socket->connect(nullptr, server.getAddress(), 30);
   evb.loop();
   Mock::VerifyAndClearExpectations(cb.get());

folly/io/async/test/MockAsyncSocketObserver.h

@@ -48,6 +48,7 @@ class MockAsyncSocketLifecycleObserver : public AsyncSocket::LifecycleObserver {
 
   // additional handlers specific to AsyncSocket::LifecycleObserver
   MOCK_METHOD1(fdDetachMock, void(AsyncSocket*));
+  MOCK_METHOD1(fdAttachMock, void(AsyncSocket*));
   MOCK_METHOD2(moveMock, void(AsyncSocket*, AsyncSocket*));
 
  private:
@@ -79,6 +80,7 @@ class MockAsyncSocketLifecycleObserver : public AsyncSocket::LifecycleObserver {
     byteEventsUnavailableMock(trans, ex);
   }
   void fdDetach(AsyncSocket* sock) noexcept override { fdDetachMock(sock); }
+  void fdAttach(AsyncSocket* sock) noexcept override { fdAttachMock(sock); }
   void move(AsyncSocket* olds, AsyncSocket* news) noexcept override {
     moveMock(olds, news);
   }