Fixed unnecessary condition. Added more atomics (data race fix)

Haivision · Aug 26, 2024 · 50da652 · 50da652
1 parent ba78d54
commit 50da652
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 11 deletions.
diff --git a/srtcore/api.h b/srtcore/api.h
@@ -486,7 +486,11 @@ class CUDTUnited
         bool acquire(CUDTUnited& glob, CUDTSocket* s)
         {
             if (s == NULL)
+            {
+                socket = NULL;
                 return false;
+            }
+
             const bool caught = glob.acquireSocket(s);
             socket = caught ? s : NULL;
             return caught;

diff --git a/srtcore/core.cpp b/srtcore/core.cpp
@@ -3653,7 +3653,7 @@ void srt::CUDT::startConnect(const sockaddr_any& serv_addr, int32_t forced_isn)
     // We can't record this address yet until the cookie-confirmation is done, for safety reasons.
     sockaddr_any use_source_adr(serv_addr.family());
 
-    while (!m_bClosing)
+    while (!m_bClosing && !m_bBroken)
     {
         const steady_clock::time_point local_tnow = steady_clock::now();
         const steady_clock::duration tdiff = local_tnow - m_tsLastReqTime.load();
@@ -3815,12 +3815,6 @@ void srt::CUDT::startConnect(const sockaddr_any& serv_addr, int32_t forced_isn)
             // listener should respond with HS_VERSION_SRT1, if it is HSv5 capable.
         }
 
-        // The queue could have been kicked by the close() API call,
-        // if so, interrupt immediately.
-        if (m_bClosing || m_bBroken)
-            break;
-
-
         HLOGC(cnlog.Debug,
               log << CONID() << "startConnect: timeout from Q:recvfrom, looping again; cst=" << ConnectStatusStr(cst));
 
@@ -6868,7 +6862,7 @@ int srt::CUDT::sendmsg2(const char *data, int len, SRT_MSGCTRL& w_mctrl)
                 << " DATA SIZE: " << size << " sched-SEQUENCE: " << seqno
                 << " STAMP: " << BufferStamp(data, size));
 
-        if (w_mctrl.srctime && w_mctrl.srctime < count_microseconds(m_stats.tsStartTime.time_since_epoch()))
+        if (w_mctrl.srctime && w_mctrl.srctime < count_microseconds(m_stats.tsStartTime.load().time_since_epoch()))
         {
             LOGC(aslog.Error,
                 log << CONID() << "Wrong source time was provided. Sending is rejected.");
@@ -11875,7 +11869,7 @@ int64_t srt::CUDT::socketStartTime(SRTSOCKET u)
     if (!s)
         return APIError(MJ_NOTSUP, MN_SIDINVAL);
 
-    return count_microseconds(s->core().m_stats.tsStartTime.time_since_epoch());
+    return count_microseconds(s->core().m_stats.tsStartTime.load().time_since_epoch());
 }
 
 bool srt::CUDT::runAcceptHook(CUDT *acore, const sockaddr* peer, const CHandShake& hs, const CPacket& hspkt)

diff --git a/srtcore/core.h b/srtcore/core.h
@@ -1184,7 +1184,7 @@ class CUDT
 private: // Trace
     struct CoreStats
     {
-        time_point tsStartTime;             // timestamp when the UDT entity is started
+        atomic_time_point tsStartTime;      // timestamp when the UDT entity is started
         stats::Sender sndr;                 // sender statistics
         stats::Receiver rcvr;               // receiver statistics
 

diff --git a/test/test_common.cpp b/test/test_common.cpp
@@ -129,7 +129,7 @@ TEST(SRTAPI, RapidClose)
     SRTSOCKET sock = srt_create_socket();
     std::condition_variable cv_start;
     std::mutex cvm;
-    bool started = false, ended = false;
+    sync::atomic<bool> started(false), ended(false);
 
     std::thread connect_thread([&sock, &cv_start, &started, &ended] {
         started = true;