chenBright commented on code in PR #3599:
URL: https://github.com/apache/brpc/pull/3599#discussion_r4267084544


##########
test/endpoint_unittest.cpp:
##########
@@ -500,119 +508,180 @@ TEST(EndPointTest, endpoint_concurrency) {
     }
 }
 
-const char* g_hostname1 = "github.com";
-const char* g_hostname2 = "baidu.com";
 TEST(EndPointTest, tcp_connect) {
-    butil::EndPoint ep1;
-    butil::EndPoint ep2;
-    ASSERT_EQ(0, butil::hostname2endpoint(g_hostname1, 80, &ep1));
-    ASSERT_EQ(0, butil::hostname2endpoint(g_hostname2, 80, &ep2));
+    butil::EndPoint ep;
+    ASSERT_EQ(0, butil::str2endpoint("127.0.0.1:0", &ep));
+    butil::fd_guard listener(butil::tcp_listen(ep));
+    ASSERT_LE(0, listener) << "errno=" << errno;
+    ASSERT_EQ(0, butil::get_local_side(listener, &ep));
+    // The local TCP handshake completes without a concurrent accept thread.
+    // Drain each connection before starting the next to keep the queue empty.
     {
-        butil::fd_guard sockfd(butil::tcp_connect(ep1, nullptr));
+        butil::fd_guard sockfd(butil::tcp_connect(ep, nullptr));
         ASSERT_LE(0, sockfd) << "errno=" << errno;
+        butil::fd_guard accepted(accept(listener, nullptr, nullptr));
+        ASSERT_LE(0, accepted) << "errno=" << errno;
     }
     {
-        butil::fd_guard sockfd(butil::tcp_connect(ep1, nullptr, 1000));
+        butil::fd_guard sockfd(butil::tcp_connect(ep, nullptr, 1000));
         ASSERT_LE(0, sockfd) << "errno=" << errno;
-    }
-    {
-        butil::fd_guard sockfd(butil::tcp_connect(ep2, nullptr, 1));
-        ASSERT_EQ(-1, sockfd) << "errno=" << errno;
-        ASSERT_EQ(ETIMEDOUT, errno);
-    }
-
-    {
-        struct sockaddr_storage serv_addr{};
-        socklen_t serv_addr_size = 0;
-        ASSERT_EQ(0, endpoint2sockaddr(ep1, &serv_addr, &serv_addr_size));
-        butil::fd_guard sockfd(socket(serv_addr.ss_family, SOCK_STREAM, 0));
-        ASSERT_LE(0, sockfd);
-        bool is_blocking = butil::is_blocking(sockfd);
-        ASSERT_EQ(0, butil::pthread_timed_connect(
-            sockfd, (struct sockaddr*) &serv_addr, serv_addr_size, nullptr));
-        ASSERT_EQ(is_blocking, butil::is_blocking(sockfd));
+        butil::fd_guard accepted(accept(listener, nullptr, nullptr));
+        ASSERT_LE(0, accepted) << "errno=" << errno;
     }
 
-    {
-        struct sockaddr_storage serv_addr{};
-        socklen_t serv_addr_size = 0;
-        ASSERT_EQ(0, endpoint2sockaddr(ep2, &serv_addr, &serv_addr_size));
-        butil::fd_guard sockfd(socket(serv_addr.ss_family, SOCK_STREAM, 0));
-        ASSERT_LE(0, sockfd);
-        bool is_blocking = butil::is_blocking(sockfd);
-        // In most cases, 1 millisecond will result in a connection timeout.
-        timespec abstime = butil::milliseconds_from_now(1);
-        const int rc = butil::pthread_timed_connect(
-            sockfd, (struct sockaddr*) &serv_addr,
-            serv_addr_size, &abstime);
-        ASSERT_EQ(-1, rc);
-        ASSERT_EQ(ETIMEDOUT, errno);
-        ASSERT_EQ(is_blocking, butil::is_blocking(sockfd));
+    sockaddr_storage serv_addr{};
+    socklen_t serv_addr_size = 0;
+    ASSERT_EQ(0, endpoint2sockaddr(ep, &serv_addr, &serv_addr_size));
+    for (bool timed : {false, true}) {
+        for (bool blocking : {false, true}) {
+            butil::fd_guard sockfd(socket(serv_addr.ss_family, SOCK_STREAM, 
0));
+            ASSERT_LE(0, sockfd);
+            if (!blocking) {
+                ASSERT_EQ(0, butil::make_non_blocking(sockfd));
+            }
+            timespec deadline = butil::seconds_from_now(1);
+            ASSERT_EQ(0, butil::pthread_timed_connect(
+                sockfd, reinterpret_cast<sockaddr*>(&serv_addr), 
serv_addr_size,
+                timed ? &deadline : nullptr)) << "errno=" << errno;
+            ASSERT_EQ(blocking, butil::is_blocking(sockfd));
+            ASSERT_EQ(0, butil::is_connected(sockfd));
+            butil::fd_guard accepted(accept(listener, nullptr, nullptr));
+            ASSERT_LE(0, accepted) << "errno=" << errno;
+        }
     }
 }
 
-bool g_connect_startd = false;
-
-void TestConnectInterruptImpl(bool timed) {
-    butil::EndPoint ep;
-    ASSERT_EQ(0, butil::hostname2endpoint(g_hostname1, 80, &ep));
+#if defined(OS_LINUX) || defined(OS_MACOSX)
+int WaitForPipe(int fd, const timespec* abstime) {
+#if defined(OS_LINUX)
+    return butil::pthread_fd_wait(fd, EPOLLIN, abstime);
+#else
+    // Match the unsigned event encoding used by pthread_timed_connect.
+    return butil::pthread_fd_wait(
+        fd, static_cast<unsigned>(EVFILT_READ), abstime);
+#endif
+}
 
-    struct sockaddr_storage serv_addr{};
-    socklen_t serv_addr_size = 0;
-    ASSERT_EQ(0, endpoint2sockaddr(ep, &serv_addr, &serv_addr_size));
-    butil::fd_guard sockfd(socket(serv_addr.ss_family, SOCK_STREAM, 0));
-    ASSERT_LE(0, sockfd);
-
-    int rc;
-    if (timed) {
-        int64_t start_ms = butil::cpuwide_time_ms();
-        butil::tcp_connect(ep, nullptr);
-        int64_t connect_ms = butil::cpuwide_time_ms() - start_ms;
-        LOG(INFO) << "Connect to " << ep << ", cost " << connect_ms << "ms";
-
-        timespec abstime = butil::milliseconds_from_now(connect_ms * 10);
-        rc = butil::pthread_timed_connect(
-            sockfd, (struct sockaddr*) &serv_addr,
-            serv_addr_size, &abstime);
-    } else {
-        rc = butil::pthread_timed_connect(
-            sockfd, (struct sockaddr*) &serv_addr,
-            serv_addr_size, nullptr);
-    }
-    ASSERT_EQ(0, rc) << "errno=" << errno;
-    ASSERT_EQ(0, butil::is_connected(sockfd));
+TEST(EndPointTest, fd_wait_timeout) {
+    int fds[2];
+    ASSERT_EQ(0, pipe(fds));
+    butil::fd_guard reader(fds[0]);
+    butil::fd_guard writer(fds[1]);
+    // An empty pipe tests the deadline without TCP queue or network 
assumptions.
+    timespec deadline = butil::milliseconds_from_now(300);

Review Comment:
   Declined. The test checks the return value and ETIMEDOUT, with no 
elapsed-time assertion. If scheduling delays the call until after the deadline, 
the implementation still returns ETIMEDOUT.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to