[core] Fix for srt_connect() in the blocking mode

srtcore/core.cpp

@@ -3155,6 +3155,7 @@ void CUDT::startConnect(const sockaddr* serv_addr, int32_t forced_isn)
 
     if (e.getErrorCode() != 0)
     {
+        m_bConnecting = false;
         // The process is to be abnormally terminated, remove the connector
         // now because most likely no other processing part has done anything with it.
         m_pRcvQueue->removeConnector(m_SocketID);

test/test_connection_timeout.cpp

@@ -13,10 +13,80 @@ extern "C" {
 #endif
 
 #define INC__WIN_WINTIME // exclude gettimeofday from srt headers
+
+#else
+typedef int SOCKET;
+#define INVALID_SOCKET ((SOCKET)-1)
+#define closesocket close
 #endif
 
+#include"platform_sys.h"
 #include "srt.h"
 
+using namespace std;
+
+
+class TestConnectionTimeout
+    : public ::testing::Test
+{
+protected:
+    TestConnectionTimeout()
+    {
+        // initialization code here
+    }
+
+    ~TestConnectionTimeout()
+    {
+        // cleanup any pending stuff, but no exceptions allowed
+    }
+
+protected:
+
+    // SetUp() is run immediately before a test starts.
+    void SetUp() override
+    {
+        ASSERT_EQ(srt_startup(), 0);
+
+        m_sa.sin_family = AF_INET;
+        m_sa.sin_addr.s_addr = INADDR_ANY;
+        m_udp_sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+        ASSERT_NE(m_udp_sock, -1);
+
+        // Find unused a port not used by any other service.
+        // Otherwise srt_connect may actually connect.
+        int bind_res = -1;
+        const sockaddr* psa = reinterpret_cast<const sockaddr*>(&m_sa);
+        for (int port = 5000; port <= 5555; ++port)
+        {
+            m_sa.sin_port = htons(port);
+            bind_res = ::bind(m_udp_sock, psa, sizeof m_sa);
+            if (bind_res >= 0)
+            {
+                cerr << "Running test on port " << port << "\n";
+                break;
+            }
+        }
+
+        ASSERT_GE(bind_res, 0);
+        ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &m_sa.sin_addr), 1);
+    }
+
+    void TearDown() override
+    {
+        // Code here will be called just after the test completes.
+        // OK to throw exceptions from here if needed.
+        ASSERT_NE(closesocket(m_udp_sock), -1);
+        srt_cleanup();
+    }
+
+protected:
+
+    SOCKET m_udp_sock = INVALID_SOCKET;
+    sockaddr_in m_sa = {};
+
+};
+
+
 
 /**
  * The test creates a socket and tries to connect to a localhost port 5555
@@ -28,10 +98,9 @@ extern "C" {
  *
  * @remarks  Inspired by Max Tomilov (maxtomilov) in issue #468
 */
-TEST(Core, ConnectionTimeout) {
-    ASSERT_EQ(srt_startup(), 0);
+TEST_F(TestConnectionTimeout, Nonblocking) {
 
-    const SRTSOCKET client_sock = srt_socket(AF_INET, SOCK_DGRAM, 0);
+    const SRTSOCKET client_sock = srt_create_socket();
     ASSERT_GT(client_sock, 0);    // socket_id should be > 0
 
     // First let's check the default connection timeout value.
@@ -57,15 +126,8 @@ TEST(Core, ConnectionTimeout) {
     const int epoll_out = SRT_EPOLL_OUT | SRT_EPOLL_ERR;
     ASSERT_NE(srt_epoll_add_usock(pollid, client_sock, &epoll_out), SRT_ERROR);
 
-    sockaddr_in sa;
-    memset(&sa, 0, sizeof sa);
-    sa.sin_family = AF_INET;
-    sa.sin_port = htons(5555);
-
-    ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1);
-
-    sockaddr* psa = (sockaddr*)&sa;
-    ASSERT_NE(srt_connect(client_sock, psa, sizeof sa), SRT_ERROR);
+    const sockaddr* psa = reinterpret_cast<const sockaddr*>(&m_sa);
+    ASSERT_NE(srt_connect(client_sock, psa, sizeof m_sa), SRT_ERROR);
 
     // Socket readiness for connection is checked by polling on WRITE allowed sockets.
     {
@@ -105,5 +167,49 @@ TEST(Core, ConnectionTimeout) {
     EXPECT_EQ(srt_epoll_remove_usock(pollid, client_sock), SRT_SUCCESS);
     EXPECT_EQ(srt_close(client_sock), SRT_SUCCESS);
     (void)srt_epoll_release(pollid);
-    (void)srt_cleanup();
 }
+
+/**
+ * The test creates a socket and tries to connect to a localhost port 5555
+ * in a blocking mode. The srt_connect function is expected to return
+ * SRT_ERROR, and the error_code should be SRT_ENOSERVER, meaning a
+ * connection timeout.
+ * This test is a regression test for an issue described in PR #833.
+ * Under certain conditions m_bConnecting variable on a socket
+ * might not be reset to false after a connection attempt has failed.
+ * In that case any further call to srt_connect will return SRT_ECONNSOCK:
+ * Operation not supported: Cannot do this operation on a CONNECTED socket
+ *
+*/
+TEST_F(TestConnectionTimeout, BlockingLoop)
+{
+    const SRTSOCKET client_sock = srt_create_socket();
+    ASSERT_GT(client_sock, 0);    // socket_id should be > 0
+
+    // Set connection timeout to 999 ms to reduce the test execution time.
+    // Also need to hit a time point between two threads:
+    // srt_connect will check TTL every second,
+    // CRcvQueue::worker will wait on a socket for 10 ms.
+    // Need to have a condition, when srt_connect will process the timeout.
+    const int connection_timeout_ms = 999;
+    EXPECT_EQ(srt_setsockopt(client_sock, 0, SRTO_CONNTIMEO, &connection_timeout_ms, sizeof connection_timeout_ms), SRT_SUCCESS);
+
+    const sockaddr* psa = reinterpret_cast<const sockaddr*>(&m_sa);
+    for (int i = 0; i < 10; ++i)
+    {
+        EXPECT_EQ(srt_connect(client_sock, psa, sizeof m_sa), SRT_ERROR);
+
+        const int error_code = srt_getlasterror(nullptr);
+        EXPECT_EQ(error_code, SRT_ENOSERVER);
+        if (error_code != SRT_ENOSERVER)
+        {
+            cerr << "Connection attempt no. " << i << " resulted with: "
+                << error_code << " " << srt_getlasterror_str() << "\n";
+            break;
+        }
+    }
+
+    EXPECT_EQ(srt_close(client_sock), SRT_SUCCESS);
+}
+
+