From c88ff8882442190d94b60aab39a9266f1c88debf Mon Sep 17 00:00:00 2001
From: Sektor van Skijlen <ethouris@gmail.com>
Date: Tue, 11 Aug 2020 09:52:21 +0200
Subject: [PATCH] [core] Added proper error handling around sync resources
 (#1393)

---
 srtcore/api.cpp   | 62 +++++++++++++++++++++++++++++------------------
 srtcore/cache.h   |  1 +
 srtcore/core.cpp  | 11 ++++++++-
 srtcore/epoll.cpp |  1 +
 srtcore/sync.cpp  |  6 ++++-
 srtcore/window.h  |  1 +
 6 files changed, 57 insertions(+), 25 deletions(-)

diff --git a/srtcore/api.cpp b/srtcore/api.cpp
index c3e4376d2..01d4e4865 100644
--- a/srtcore/api.cpp
+++ b/srtcore/api.cpp
@@ -199,6 +199,9 @@ m_ClosedSockets()
    m_SocketIDGenerator = 1 + int(MAX_SOCKET_VAL * rand1_0);
    m_SocketIDGenerator_init = m_SocketIDGenerator;
 
+   // XXX An unlikely exception thrown from the below calls
+   // might destroy the application before `main`. This shouldn't
+   // be a problem in general.
    setupMutex(m_GlobControlLock, "GlobControl");
    setupMutex(m_IDLock, "ID");
    setupMutex(m_InitLock, "Init");
@@ -257,8 +260,15 @@ int CUDTUnited::startup()
 
    m_bClosing = false;
 
-   setupMutex(m_GCStopLock, "GCStop");
-   setupCond(m_GCStopCond, "GCStop");
+   try
+   {
+       setupMutex(m_GCStopLock, "GCStop");
+       setupCond(m_GCStopCond, "GCStop");
+   }
+   catch (...)
+   {
+       return -1;
+   }
    if (!StartThread(m_GCThread, garbageCollect, this, "SRT:GC"))
       return -1;
 
@@ -2584,6 +2594,26 @@ void CUDTUnited::updateMux(
            // port was specified as 0.
            m.m_pChannel->open(addr);
        }
+
+       sockaddr_any sa;
+       m.m_pChannel->getSockAddr((sa));
+       m.m_iPort = sa.hport();
+       s->m_SelfAddr = sa; // Will be also completed later, but here it's needed for later checks
+
+       m.m_pTimer = new CTimer;
+
+       m.m_pSndQueue = new CSndQueue;
+       m.m_pSndQueue->init(m.m_pChannel, m.m_pTimer);
+       m.m_pRcvQueue = new CRcvQueue;
+       m.m_pRcvQueue->init(
+               32, s->m_pUDT->maxPayloadSize(), m.m_iIPversion, 1024,
+               m.m_pChannel, m.m_pTimer);
+
+       m_mMultiplexer[m.m_iID] = m;
+
+       s->m_pUDT->m_pSndQueue = m.m_pSndQueue;
+       s->m_pUDT->m_pRcvQueue = m.m_pRcvQueue;
+       s->m_iMuxID = m.m_iID;
    }
    catch (CUDTException& e)
    {
@@ -2591,26 +2621,12 @@ void CUDTUnited::updateMux(
       delete m.m_pChannel;
       throw;
    }
-
-   sockaddr_any sa;
-   m.m_pChannel->getSockAddr((sa));
-   m.m_iPort = sa.hport();
-   s->m_SelfAddr = sa; // Will be also completed later, but here it's needed for later checks
-
-   m.m_pTimer = new CTimer;
-
-   m.m_pSndQueue = new CSndQueue;
-   m.m_pSndQueue->init(m.m_pChannel, m.m_pTimer);
-   m.m_pRcvQueue = new CRcvQueue;
-   m.m_pRcvQueue->init(
-      32, s->m_pUDT->maxPayloadSize(), m.m_iIPversion, 1024,
-      m.m_pChannel, m.m_pTimer);
-
-   m_mMultiplexer[m.m_iID] = m;
-
-   s->m_pUDT->m_pSndQueue = m.m_pSndQueue;
-   s->m_pUDT->m_pRcvQueue = m.m_pRcvQueue;
-   s->m_iMuxID = m.m_iID;
+   catch (...)
+   {
+      m.m_pChannel->close();
+      delete m.m_pChannel;
+      throw CUDTException(MJ_SYSTEMRES, MN_MEMORY, 0);
+   }
 
    HLOGF(mglog.Debug, 
       "creating new multiplexer for port %i\n", m.m_iPort);
@@ -2827,7 +2843,7 @@ SRTSOCKET CUDT::createGroup(SRT_GROUP_TYPE gt)
     {
         return APIError(e);
     }
-    catch (const std::bad_alloc& e)
+    catch (...)
     {
         return APIError(MJ_SYSTEMRES, MN_MEMORY, 0);
     }
diff --git a/srtcore/cache.h b/srtcore/cache.h
index 9ee0c65ef..afaa404e0 100644
--- a/srtcore/cache.h
+++ b/srtcore/cache.h
@@ -83,6 +83,7 @@ template<typename T> class CCache
    m_iCurrSize(0)
    {
       m_vHashPtr.resize(m_iHashSize);
+      // Exception: -> CUDTUnited ctor
       srt::sync::setupMutex(m_Lock, "Cache");
    }
 
diff --git a/srtcore/core.cpp b/srtcore/core.cpp
index 4b3bfca7e..41a392e2e 100644
--- a/srtcore/core.cpp
+++ b/srtcore/core.cpp
@@ -3607,7 +3607,16 @@ SRTSOCKET CUDT::makeMePeerOf(SRTSOCKET peergroup, SRT_GROUP_TYPE gtp, uint32_t l
     }
     else
     {
-        gp = &newGroup(gtp);
+        try
+        {
+            gp = &newGroup(gtp);
+        }
+        catch (...)
+        {
+            // Expected exceptions are only those referring to system resources
+            return -1;
+        }
+
         if (!gp->applyFlags(link_flags, m_SrtHsSide))
         {
             // Wrong settings. Must reject. Delete group.
diff --git a/srtcore/epoll.cpp b/srtcore/epoll.cpp
index 2c7f98e5a..9a3204041 100644
--- a/srtcore/epoll.cpp
+++ b/srtcore/epoll.cpp
@@ -85,6 +85,7 @@ using namespace srt_logging;
 CEPoll::CEPoll():
 m_iIDSeed(0)
 {
+   // Exception -> CUDTUnited ctor.
    setupMutex(m_EPollLock, "EPoll");
 }
 
diff --git a/srtcore/sync.cpp b/srtcore/sync.cpp
index 65c27ecf0..c4c79d27c 100644
--- a/srtcore/sync.cpp
+++ b/srtcore/sync.cpp
@@ -236,7 +236,11 @@ std::string srt::sync::FormatTimeSys(const steady_clock::time_point& timestamp)
 #if !defined(ENABLE_STDCXX_SYNC)
 srt::sync::Mutex::Mutex()
 {
-    pthread_mutex_init(&m_mutex, NULL);
+    const int err = pthread_mutex_init(&m_mutex, 0);
+    if (err)
+    {
+        throw CUDTException(MJ_SYSTEMRES, MN_MEMORY, 0);
+    }
 }
 
 srt::sync::Mutex::~Mutex()
diff --git a/srtcore/window.h b/srtcore/window.h
index cbe6ad717..20b2374dc 100644
--- a/srtcore/window.h
+++ b/srtcore/window.h
@@ -148,6 +148,7 @@ class CPktTimeWindow: CPktTimeWindowTools
         m_tsProbeTime(),
         m_Probe1Sequence(SRT_SEQNO_NONE)
     {
+        // Exception: up to CUDT ctor
         srt::sync::setupMutex(m_lockPktWindow, "PktWindow");
         srt::sync::setupMutex(m_lockProbeWindow, "ProbeWindow");
         CPktTimeWindowTools::initializeWindowArrays(m_aPktWindow, m_aProbeWindow, m_aBytesWindow, ASIZE, PSIZE);