[core] Unguarded access to epolldesc from group sender code (#1846)

Haivision · Mar 8, 2021 · 47e477b · 47e477b
1 parent de54e65
commit 47e477b
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 12 deletions.
diff --git a/srtcore/epoll.cpp b/srtcore/epoll.cpp
@@ -808,6 +808,12 @@ int CEPoll::swait(CEPollDesc& d, map<SRTSOCKET, int>& st, int64_t msTimeOut, boo
     return 0;
 }
 
+bool CEPoll::empty(CEPollDesc& d)
+{
+    ScopedLock lg (m_EPollLock);
+    return d.watch_empty();
+}
+
 int CEPoll::release(const int eid)
 {
    ScopedLock pg(m_EPollLock);

diff --git a/srtcore/epoll.h b/srtcore/epoll.h
@@ -60,7 +60,7 @@ modified by
 #include "udt.h"
 
 
-struct CEPollDesc
+class CEPollDesc
 {
    const int m_iID;                                // epoll ID
 
@@ -143,8 +143,6 @@ struct CEPollDesc
 std::string DisplayEpollWatch();
 #endif
 
-private:
-
    /// Sockets that are subscribed for events in this eid.
    ewatch_t m_USockWatchState;
 
@@ -159,7 +157,10 @@ std::string DisplayEpollWatch();
 
    enotice_t::iterator nullNotice() { return m_USockEventNotice.end(); }
 
-public:
+   // Only CEPoll class should have access to it.
+   // Guarding private access to the class is not necessary
+   // within the epoll module.
+   friend class CEPoll;
 
    CEPollDesc(int id, int localID)
        : m_iID(id)
@@ -423,6 +424,9 @@ friend class CRendezvousQueue;
    /// @retval >=0 number of ready sockets (actually size of `st`)
    int swait(CEPollDesc& d, fmap_t& st, int64_t msTimeOut, bool report_by_exception = true);
 
+   /// Empty subscription check - for internal use only.
+   bool empty(CEPollDesc& d);
+
    /// Reports which events are ready on the given socket.
    /// @param mp socket event map retirned by `swait`
    /// @param sock which socket to ask

diff --git a/srtcore/group.cpp b/srtcore/group.cpp
@@ -1387,7 +1387,7 @@ int CUDTGroup::sendBroadcast(const char* buf, int len, SRT_MSGCTRL& w_mc)
         // at the connecting stage.
         CEPoll::fmap_t sready;
 
-        if (m_SndEpolld->watch_empty())
+        if (m_pGlobal->m_EPoll.empty(*m_SndEpolld))
         {
             // Sanity check - weird pending reported.
             LOGC(gslog.Error,
@@ -3486,18 +3486,14 @@ void CUDTGroup::send_CheckPendingSockets(const vector<SRTSOCKET>& pendingSockets
     // at the connecting stage.
     CEPoll::fmap_t sready;
 
-    if (m_SndEpolld->watch_empty())
+    if (m_pGlobal->m_EPoll.empty(*m_SndEpolld))
     {
         // Sanity check - weird pending reported.
         LOGC(gslog.Error, log << "grp/send*: IPE: reported pending sockets, but EID is empty - wiping pending!");
         copy(pendingSockets.begin(), pendingSockets.end(), back_inserter(w_wipeme));
     }
     else
     {
-        // Some sockets could have been closed in the meantime.
-        if (m_SndEpolld->watch_empty())
-            throw CUDTException(MJ_CONNECTION, MN_CONNLOST, 0);
-
         {
             InvertedLock ug(m_GroupLock);
             m_pGlobal->m_EPoll.swait(
@@ -3510,6 +3506,11 @@ void CUDTGroup::send_CheckPendingSockets(const vector<SRTSOCKET>& pendingSockets
             throw CUDTException(MJ_CONNECTION, MN_CONNLOST, 0);
         }
 
+        // Some sockets could have been closed in the meantime.
+        if (m_pGlobal->m_EPoll.empty(*m_SndEpolld))
+            throw CUDTException(MJ_CONNECTION, MN_CONNLOST, 0);
+
+
         HLOGC(gslog.Debug, log << "grp/send*: RDY: " << DisplayEpollResults(sready));
 
         // sockets in EX: should be moved to w_wipeme.
@@ -3633,7 +3634,7 @@ void CUDTGroup::sendBackup_RetryWaitBlocked(const vector<gli_t>& unstableLinks,
     m_pGlobal->m_EPoll.update_events(id(), m_sPollID, SRT_EPOLL_OUT, false);
     m_pGlobal->m_EPoll.update_events(id(), m_sPollID, SRT_EPOLL_ERR, true);
 
-    if (m_SndEpolld->watch_empty())
+    if (m_pGlobal->m_EPoll.empty(*m_SndEpolld))
     {
         // wipeme wiped, pending sockets checked, it can only mean that
         // all sockets are broken.
@@ -3669,7 +3670,7 @@ void CUDTGroup::sendBackup_RetryWaitBlocked(const vector<gli_t>& unstableLinks,
 RetryWaitBlocked:
     {
         // Some sockets could have been closed in the meantime.
-        if (m_SndEpolld->watch_empty())
+        if (m_pGlobal->m_EPoll.empty(*m_SndEpolld))
         {
             HLOGC(gslog.Debug, log << "grp/sendBackup: no more sockets available for sending - group broken");
             throw CUDTException(MJ_CONNECTION, MN_CONNLOST, 0);