Skip to content

Commit

Permalink
[core][MAINT] Removed unused and swelling m_pAcceptSockets field + re…
Browse files Browse the repository at this point in the history 
…fax (#1740)
  • Loading branch information
@ethouris
ethouris authored Jan 15, 2021
1 parent 648e8b5 commit 8b4c8cd
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 64 deletions.
75 changes: 15 additions & 60 deletions srtcore/api.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,6 @@ CUDTSocket::~CUDTSocket()
delete m_pUDT;
m_pUDT = NULL;

delete m_pQueuedSockets;
delete m_pAcceptSockets;

releaseMutex(m_AcceptLock);
releaseCond(m_AcceptCond);
releaseMutex(m_ControlLock);
Expand Down Expand Up @@ -162,7 +159,7 @@ bool CUDTSocket::readReady()
return true;
if (m_pUDT->m_bListening)
{
return m_pQueuedSockets->size() > 0;
return m_QueuedSockets.size() > 0;
}

return broken();
Expand Down Expand Up @@ -518,8 +515,7 @@ int CUDTUnited::newConnection(const SRTSOCKET listen, const sockaddr_any& peer,
ns->setClosed();

ScopedLock acceptcg(ls->m_AcceptLock);
ls->m_pQueuedSockets->erase(ns->m_SocketID);
ls->m_pAcceptSockets->erase(ns->m_SocketID);
ls->m_QueuedSockets.erase(ns->m_SocketID);
}
else
{
Expand Down Expand Up @@ -551,7 +547,7 @@ int CUDTUnited::newConnection(const SRTSOCKET listen, const sockaddr_any& peer,
}

// exceeding backlog, refuse the connection request
if (ls->m_pQueuedSockets->size() >= ls->m_uiBackLog)
if (ls->m_QueuedSockets.size() >= ls->m_uiBackLog)
{
w_error = SRT_REJ_BACKLOG;
LOGC(cnlog.Note, log << "newConnection: listen backlog=" << ls->m_uiBackLog << " EXCEEDED");
Expand Down Expand Up @@ -778,7 +774,7 @@ int CUDTUnited::newConnection(const SRTSOCKET listen, const sockaddr_any& peer,
enterCS(ls->m_AcceptLock);
try
{
ls->m_pQueuedSockets->insert(ns->m_SocketID);
ls->m_QueuedSockets.insert(ns->m_SocketID);
}
catch (...)
{
Expand Down Expand Up @@ -806,10 +802,6 @@ int CUDTUnited::newConnection(const SRTSOCKET listen, const sockaddr_any& peer,
{
HLOGC(cnlog.Debug, log << "ACCEPT: new socket @" << ns->m_SocketID
<< " NOT submitted to acceptance, another socket in the group is already connected");
{
ScopedLock cg (ls->m_AcceptLock);
ls->m_pAcceptSockets->insert(ls->m_pAcceptSockets->end(), ns->m_SocketID);
}

// acknowledge INTERNAL users waiting for new connections on the listening socket
// that are reported when a new socket is connected within an already connected group.
Expand Down Expand Up @@ -1013,21 +1005,6 @@ int CUDTUnited::listen(const SRTSOCKET u, int backlog)

s->m_uiBackLog = backlog;

try
{
s->m_pQueuedSockets = new set<SRTSOCKET>;
s->m_pAcceptSockets = new set<SRTSOCKET>;
}
catch (...)
{
delete s->m_pQueuedSockets;
delete s->m_pAcceptSockets;

// XXX Translated std::bad_alloc into CUDTException specifying
// memory allocation failure...
throw CUDTException(MJ_SYSTEMRES, MN_MEMORY, 0);
}

// [[using assert(s->m_Status == OPENED)]]; // (still, unchanged)

s->m_pUDT->setListenState(); // propagates CUDTException,
Expand Down Expand Up @@ -1119,30 +1096,11 @@ SRTSOCKET CUDTUnited::accept(const SRTSOCKET listen, sockaddr* pw_addr, int* pw_
// This socket has been closed.
accepted = true;
}
else if (ls->m_pQueuedSockets->size() > 0)
else if (ls->m_QueuedSockets.size() > 0)
{
// XXX REFACTORING REQUIRED HERE!
// Actually this should at best be something like that:
// set<SRTSOCKET>::iterator b = ls->m_pQueuedSockets->begin();
// u = *b;
// ls->m_pQueuedSockets->erase(b);
// ls->m_pAcceptSockets->insert(u);
//
// It is also questionable why m_pQueuedSockets should be of type 'set'.
// There's no quick-searching capabilities of that container used anywhere except
// checkBrokenSockets and garbageCollect, which aren't performance-critical,
// whereas it's mainly used for getting the first element and iterating
// over elements, which is slow in case of std::set. It's also doubtful
// as to whether the sorting capability of std::set is properly used;
// the first is taken here, which is actually the socket with lowest
// possible descriptor value (as default operator< and ascending sorting
// used for std::set<SRTSOCKET> where SRTSOCKET=int).
//
// Consider using std::list or std::vector here.

u = *(ls->m_pQueuedSockets->begin());
ls->m_pAcceptSockets->insert(ls->m_pAcceptSockets->end(), u);
ls->m_pQueuedSockets->erase(ls->m_pQueuedSockets->begin());
set<SRTSOCKET>::iterator b = ls->m_QueuedSockets.begin();
u = *b;
ls->m_QueuedSockets.erase(b);
accepted = true;
}
else if (!ls->m_pUDT->m_bSynRecving)
Expand All @@ -1153,7 +1111,7 @@ SRTSOCKET CUDTUnited::accept(const SRTSOCKET listen, sockaddr* pw_addr, int* pw_
if (!accepted && (ls->m_Status == SRTS_LISTENING))
accept_sync.wait();

if (ls->m_pQueuedSockets->empty())
if (ls->m_QueuedSockets.empty())
m_EPoll.update_events(listen, ls->m_pUDT->m_sPollID, SRT_EPOLL_ACCEPT, false);
}

Expand Down Expand Up @@ -2343,7 +2301,7 @@ int CUDTUnited::selectEx(
&& s->m_pUDT->m_pRcvBuffer->isRcvDataReady()
)
|| (s->m_pUDT->m_bListening
&& (s->m_pQueuedSockets->size() > 0)))
&& (s->m_QueuedSockets.size() > 0)))
{
readfds->push_back(s->m_SocketID);
++ count;
Expand Down Expand Up @@ -2696,8 +2654,7 @@ void CUDTUnited::checkBrokenSockets()
}

enterCS(ls->second->m_AcceptLock);
ls->second->m_pQueuedSockets->erase(s->m_SocketID);
ls->second->m_pAcceptSockets->erase(s->m_SocketID);
ls->second->m_QueuedSockets.erase(s->m_SocketID);
leaveCS(ls->second->m_AcceptLock);
}
}
Expand Down Expand Up @@ -2766,20 +2723,19 @@ void CUDTUnited::removeSocket(const SRTSOCKET u)
// decrease multiplexer reference count, and remove it if necessary
const int mid = s->m_iMuxID;

if (s->m_pQueuedSockets)
{
ScopedLock cg(s->m_AcceptLock);

// if it is a listener, close all un-accepted sockets in its queue
// and remove them later
for (set<SRTSOCKET>::iterator q = s->m_pQueuedSockets->begin();
q != s->m_pQueuedSockets->end(); ++ q)
for (set<SRTSOCKET>::iterator q = s->m_QueuedSockets.begin();
q != s->m_QueuedSockets.end(); ++ q)
{
sockets_t::iterator si = m_Sockets.find(*q);
if (si == m_Sockets.end())
{
// gone in the meantime
LOGC(smlog.Error, log << "removeSocket: IPE? socket @" << u
LOGC(smlog.Error, log << "removeSocket: IPE? socket @" << (*q)
<< " being queued for listener socket @" << s->m_SocketID
<< " is GONE in the meantime ???");
continue;
Expand Down Expand Up @@ -3120,8 +3076,7 @@ void* CUDTUnited::garbageCollect(void* p)
}

enterCS(ls->second->m_AcceptLock);
ls->second->m_pQueuedSockets->erase(s->m_SocketID);
ls->second->m_pAcceptSockets->erase(s->m_SocketID);
ls->second->m_QueuedSockets.erase(s->m_SocketID);
leaveCS(ls->second->m_AcceptLock);
}
self->m_Sockets.clear();
Expand Down
5 changes: 1 addition & 4 deletions srtcore/api.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ class CUDTSocket
#endif
, m_iISN(0)
, m_pUDT(NULL)
, m_pQueuedSockets(NULL)
, m_pAcceptSockets(NULL)
, m_AcceptCond()
, m_AcceptLock()
, m_uiBackLog(0)
Expand Down Expand Up @@ -127,8 +125,7 @@ class CUDTSocket

CUDT* m_pUDT; //< pointer to the UDT entity

std::set<SRTSOCKET>* m_pQueuedSockets; //< set of connections waiting for accept()
std::set<SRTSOCKET>* m_pAcceptSockets; //< set of accept()ed connections
std::set<SRTSOCKET> m_QueuedSockets; //< set of connections waiting for accept()

srt::sync::Condition m_AcceptCond; //< used to block "accept" call
srt::sync::Mutex m_AcceptLock; //< mutex associated to m_AcceptCond
Expand Down

0 comments on commit 8b4c8cd

Please sign in to comment.