Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added proper error handling around sync resources #1393

Merged
merged 5 commits into from
Aug 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 39 additions & 23 deletions srtcore/api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -2584,33 +2594,39 @@ 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)
{
m.m_pChannel->close();
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);
Expand Down Expand Up @@ -2827,7 +2843,7 @@ SRTSOCKET CUDT::createGroup(SRT_GROUP_TYPE gt)
{
return APIError(e);
}
catch (const std::bad_alloc& e)
catch (...)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why bad_alock was not enough there?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There could be some other types of exceptions thrown from the C++ standard library, as I remember. All of them qualify as IPE or at best lethal state of the system resources.

{
return APIError(MJ_SYSTEMRES, MN_MEMORY, 0);
}
Expand Down
1 change: 1 addition & 0 deletions srtcore/cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

Expand Down
11 changes: 10 additions & 1 deletion srtcore/core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 (...)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for any certain exception or any?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably only system exceptions and those freshly added here (although "added" is only in order to comply with the C++ std version).

{
// Expected exceptions are only those referring to system resources
return -1;
}

if (!gp->applyFlags(link_flags, m_SrtHsSide))
{
// Wrong settings. Must reject. Delete group.
Expand Down
1 change: 1 addition & 0 deletions srtcore/epoll.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ using namespace srt_logging;
CEPoll::CEPoll():
m_iIDSeed(0)
{
// Exception -> CUDTUnited ctor.
setupMutex(m_EPollLock, "EPoll");
}

Expand Down
6 changes: 5 additions & 1 deletion srtcore/sync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions srtcore/window.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down