[core] Fixed too early closed caller socket in background. (#1750)

- Updated documentation
- Lifted rules for a test that should result in the late break on the listener side

docs/API-functions.md

@@ -783,6 +783,11 @@ is through the epoll flag with [`SRT_EPOLL_ERR`](#SRT_EPOLL_ERR). In this case y
 also call [`srt_getrejectreason`](#srt_getrejectreason) to get the detailed reason for 
 the error, including connection timeout ([`SRT_REJ_TIMEOUT`](#SRT_REJ_TIMEOUT)).
 
+Note that in case of failure the socket is in `SRTS_CONNECTING`, not in
+`SRTS_BROKEN` state. After the failure was reported and you read any extra
+information from the socket, the socket should be manually closed using
+`srt_close` function.
+
 
 
 [:arrow_up:   Back to List of Functions & Structures](#srt-api-functions)

docs/API.md

@@ -154,6 +154,15 @@ and `remote_name` must use the same port. The peer to which this is going to con
 should call the same function, with appropriate local and remote addresses. A rendezvous
 connection means that both parties connect to one another simultaneously.
 
+**IMPORTANT**: The connection may fail, but the socket that was used for connecting
+is not automatically closed and it's also not in broken state (broken state can be
+only if a socket was first successfully connected and then broken). When using blocking
+mode, the connection failure will result in reporting an error from this function call.
+In non-blocking mode the connection failure is designated by the `SRT_EPOLL_ERR` flag
+set for this socket in the epoll container. After that failure you can read an extra
+information from the socket using `srt_getrejectreason` function, and then you should
+close the socket.
+
 ### Listener (Server) Example
 
 ```c++

srtcore/queue.cpp

@@ -1098,7 +1098,15 @@ void CRendezvousQueue::updateConnStatus(EReadStatus rst, EConnectStatus cst, con
     {
         HLOGC(cnlog.Debug, log << "updateConnStatus: COMPLETING dep objects update on failed @" << i->id);
         i->u->m_bConnecting = false;
-        i->u->updateBrokenConnection();
+
+        // DO NOT close the socket here because in this case it might be
+        // unable to get status from at the right moment. Also only member
+        // sockets should be taken care of internally - single sockets should
+        // be normally closed by the application, after it is done with them.
+
+        // app can call any UDT API to learn the connection_broken error
+        CUDT::s_UDTUnited.m_EPoll.update_events(i->u->m_SocketID, i->u->m_sPollID, SRT_EPOLL_IN | SRT_EPOLL_OUT | SRT_EPOLL_ERR, true);
+
         i->u->completeBrokenConnectionDependencies(i->errorcode);
     }
 

test/test_connection_timeout.cpp

@@ -172,20 +172,20 @@ TEST_F(TestConnectionTimeout, Nonblocking) {
 */
 TEST_F(TestConnectionTimeout, BlockingLoop)
 {
-    const sockaddr* psa = reinterpret_cast<const sockaddr*>(&m_sa);
+    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)
     {
-        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.
-        EXPECT_EQ(srt_setsockopt(client_sock, 0, SRTO_CONNTIMEO, &connection_timeout_ms, sizeof connection_timeout_ms), SRT_SUCCESS);
-
         EXPECT_EQ(srt_connect(client_sock, psa, sizeof m_sa), SRT_ERROR);
 
         const int error_code = srt_getlasterror(nullptr);
@@ -196,9 +196,9 @@ TEST_F(TestConnectionTimeout, BlockingLoop)
                 << error_code << " " << srt_getlasterror_str() << "\n";
             break;
         }
-
-        EXPECT_EQ(srt_close(client_sock), SRT_SUCCESS);
     }
+
+    EXPECT_EQ(srt_close(client_sock), SRT_SUCCESS);
 }
 
 

test/test_enforced_encryption.cpp

@@ -415,22 +415,25 @@ class TestEnforcedEncryption
                     std::this_thread::sleep_for(std::chrono::milliseconds(50));
                 } while (!caller_done);
 
-                const SRT_SOCKSTATUS status = srt_getsockstate(accepted_socket);
-                if (m_is_tracing)
-                {
-                    std::cerr << "LATE Socket state accepted: " << m_socket_state[status]
-                        << " (expected: " << m_socket_state[expect.socket_state[CHECK_SOCKET_ACCEPTED]] << ")\n";
-                }
+                // Special case when the expected state is "broken": if so, tolerate every possible
+                // socket state, just NOT LESS than SRTS_BROKEN, and also don't read any flags on that socket.
 
                 if (expect.socket_state[CHECK_SOCKET_ACCEPTED] == SRTS_BROKEN)
                 {
-                    EXPECT_TRUE(accepted_socket == -1 || status == SRTS_BROKEN || status == SRTS_CLOSED);
+                    EXPECT_GE(srt_getsockstate(accepted_socket), SRTS_BROKEN);
                 }
                 else
                 {
-                    EXPECT_EQ(status, expect.socket_state[CHECK_SOCKET_ACCEPTED]);
+                    EXPECT_EQ(srt_getsockstate(accepted_socket), expect.socket_state[CHECK_SOCKET_ACCEPTED]);
                     EXPECT_EQ(GetSocetkOption(accepted_socket, SRTO_SNDKMSTATE), expect.km_state[CHECK_SOCKET_ACCEPTED]);
                 }
+
+                if (m_is_tracing)
+                {
+                    const SRT_SOCKSTATUS status = srt_getsockstate(accepted_socket);
+                    std::cerr << "LATE Socket state accepted: " << m_socket_state[status]
+                        << " (expected: " << m_socket_state[expect.socket_state[CHECK_SOCKET_ACCEPTED]] << ")\n";
+                }
             }
         });