From 55310c247f430fba1fef975308971fddc0ccd924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Ma=C5=82ecki?= Date: Thu, 2 Jul 2020 10:54:21 +0200 Subject: [PATCH 1/3] Added proper error handling around sync resources --- srtcore/api.cpp | 61 ++++++++++++++++++++++++++++++----------------- 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(+), 24 deletions(-) diff --git a/srtcore/api.cpp b/srtcore/api.cpp index 72aa1d42f..707518c6c 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; @@ -2529,6 +2539,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(); + + 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) { @@ -2536,25 +2566,12 @@ void CUDTUnited::updateMux( delete m.m_pChannel; throw; } - - sockaddr_any sa; - m.m_pChannel->getSockAddr((sa)); - m.m_iPort = sa.hport(); - - 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); @@ -2771,7 +2788,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 04dc872cd..4cedd8558 100644 --- a/srtcore/cache.h +++ b/srtcore/cache.h @@ -83,6 +83,7 @@ template 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 1a6f61e6c..1f8f46f0a 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -3592,7 +3592,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 82c7587df..92f92f80a 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..c51e09359 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); + 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 3cbd4134d..a3845ff3c 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); From 35feb0cf4ac65e1543ae00d32722f16cabc31c1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Ma=C5=82ecki?= Date: Mon, 27 Jul 2020 17:27:03 +0200 Subject: [PATCH 2/3] CodeFactor --- srtcore/api.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/srtcore/api.cpp b/srtcore/api.cpp index 707518c6c..0ca81816c 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -2558,7 +2558,6 @@ void CUDTUnited::updateMux( 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) { From 9bb3431fccb2a51d4e6ec10b30afc9d55d23ea9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Ma=C5=82ecki?= Date: Mon, 10 Aug 2020 12:53:12 +0200 Subject: [PATCH 3/3] Followup --- srtcore/api.cpp | 2 +- srtcore/sync.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/srtcore/api.cpp b/srtcore/api.cpp index 7c9764cb0..01d4e4865 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -2598,7 +2598,7 @@ void CUDTUnited::updateMux( 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 + s->m_SelfAddr = sa; // Will be also completed later, but here it's needed for later checks m.m_pTimer = new CTimer; diff --git a/srtcore/sync.cpp b/srtcore/sync.cpp index c51e09359..c4c79d27c 100644 --- a/srtcore/sync.cpp +++ b/srtcore/sync.cpp @@ -236,7 +236,7 @@ std::string srt::sync::FormatTimeSys(const steady_clock::time_point& timestamp) #if !defined(ENABLE_STDCXX_SYNC) srt::sync::Mutex::Mutex() { - int err = pthread_mutex_init(&m_mutex, 0); + const int err = pthread_mutex_init(&m_mutex, 0); if (err) { throw CUDTException(MJ_SYSTEMRES, MN_MEMORY, 0);