Skip to content

Commit

Permalink
Fix missing increment of the instance count on every call to startup()
Browse files Browse the repository at this point in the history
Change how the implicit startup is tracked.
The extra change is needed since the startup() calls in
srt::CUDT::socket()
and
srt::CUDT::createGroup
would endlessly increment the startup counter as sockets were created
otherwise.

Force a clean() when the singleton is getting destructed.
Previously it just did a single decrement of the instance count,
which could result in attempting to exit without actually cleaning up.

fixes #3098
  • Loading branch information
mbechard committed Jan 13, 2025
1 parent 8a89a3a commit 4275d7b
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 16 deletions.
38 changes: 24 additions & 14 deletions srtcore/api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,8 @@ srt::CUDTUnited::CUDTUnited()

srt::CUDTUnited::~CUDTUnited()
{
// Call it if it wasn't called already.
// This will happen at the end of main() of the application,
// when the user didn't call srt_cleanup().
if (m_bGCStatus)
{
cleanup();
}
// force cleanup, even if the instance count is > 1
cleanup(true);

releaseMutex(m_GlobControlLock);
releaseMutex(m_IDLock);
Expand Down Expand Up @@ -233,13 +228,20 @@ string srt::CUDTUnited::CONID(SRTSOCKET sock)
return os.str();
}

int srt::CUDTUnited::startup()
int srt::CUDTUnited::startup(bool implicit)
{
ScopedLock gcinit(m_InitLock);
if (m_bGCStatus)

if (implicit && m_iInstanceCount > 0)
// [reinstate in 1.6.0] return m_iInstanceCount;
return 1;

if (m_iInstanceCount++ > 0)
// [reinstate in 1.6.0] return m_iInstanceCount;
return 1;

if (m_bGCStatus)
// [reinstate in 1.6.0] return m_iInstanceCount;
return 1;

// Global initialization code
Expand Down Expand Up @@ -268,7 +270,7 @@ int srt::CUDTUnited::startup()
return 0;
}

int srt::CUDTUnited::cleanup()
int srt::CUDTUnited::cleanup(bool force)
{
// IMPORTANT!!!
// In this function there must be NO LOGGING AT ALL. This function may
Expand All @@ -280,9 +282,14 @@ int srt::CUDTUnited::cleanup()
// executing this procedure.
ScopedLock gcinit(m_InitLock);

if (--m_iInstanceCount > 0)
return 0;
if (!force)
{
if (m_iInstanceCount == 0 || --m_iInstanceCount > 0)
// [reinstate in 1.6.0] return m_iInstanceCount;
return 0;
}

// Cleanup done before already
if (!m_bGCStatus)
return 0;

Expand All @@ -298,13 +305,16 @@ int srt::CUDTUnited::cleanup()
CSync::notify_one_relaxed(m_GCStopCond);
m_GCThread.join();

if (force)
m_iInstanceCount = 0;
m_bGCStatus = false;

// Global destruction code
#ifdef _WIN32
WSACleanup();
#endif

// Cleanup done at all.
return 0;
}

Expand Down Expand Up @@ -3469,7 +3479,7 @@ int srt::CUDT::cleanup()

SRTSOCKET srt::CUDT::socket()
{
uglobal().startup();
uglobal().startup(true);

try
{
Expand Down Expand Up @@ -3519,7 +3529,7 @@ srt::CUDTGroup& srt::CUDT::newGroup(const int type)
SRTSOCKET srt::CUDT::createGroup(SRT_GROUP_TYPE gt)
{
// Doing the same lazy-startup as with srt_create_socket()
uglobal().startup();
uglobal().startup(true);

try
{
Expand Down
15 changes: 13 additions & 2 deletions srtcore/api.h
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,14 @@ class CUDTUnited
static std::string CONID(SRTSOCKET sock);

/// initialize the UDT library.
/// @param [in] implicit should be set to true for internal implicit startup() calls.
/// @return 0 if success, otherwise -1 is returned.
int startup();
int startup(bool implicit = false);

/// release the UDT library.
/// @param [in] force cleanup regardless of the instance count.
/// @return 0 if success, otherwise -1 is returned.
int cleanup();
int cleanup(bool force = false);

/// Create a new UDT socket.
/// @param [out] pps Variable (optional) to which the new socket will be written, if succeeded
Expand Down Expand Up @@ -542,6 +544,15 @@ class CUDTUnited
int m_iInstanceCount; // number of startup() called by application
SRT_ATTR_GUARDED_BY(m_InitLock)
bool m_bGCStatus; // if the GC thread is working (true)
public:

std::pair<int, bool> getInstanceStatus()
{
sync::ScopedLock lk(m_InitLock);
return std::make_pair(m_iInstanceCount, m_bGCStatus);
}

private:

SRT_ATTR_GUARDED_BY(m_InitLock)
sync::CThread m_GCThread;
Expand Down
32 changes: 32 additions & 0 deletions test/test_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,41 @@
#include "test_env.h"
#include "utilities.h"
#include "common.h"
#include "api.h"

using namespace srt;

TEST(General, Startup)
{
// Should return 0 if it was run for the first time
// and actually performed the initialization.
EXPECT_EQ(srt_startup(), 0);

EXPECT_EQ(srt::CUDT::uglobal().getInstanceStatus(), std::make_pair(1, true));

// Every next one should return the number of nested instances
// [reinstate in 1.6.0] EXPECT_EQ(srt_startup(), 2);
srt_startup();
EXPECT_EQ(srt::CUDT::uglobal().getInstanceStatus(), std::make_pair(2, true));

// Now let's pair the first instance, should NOT execute
// [reinstate in 1.6.0] EXPECT_EQ(srt_cleanup(), 1);
srt_cleanup();

EXPECT_EQ(srt_cleanup(), 0);

// Second cleanup, should report successful cleanup even if nothing is done.
EXPECT_EQ(srt_cleanup(), 0);

// Now let's start with getting the number of instances
// from implicitly created ones.
SRTSOCKET sock = srt_create_socket();

EXPECT_EQ(srt::CUDT::uglobal().getInstanceStatus(), std::make_pair(1, true));

EXPECT_EQ(srt_close(sock), 0);
}

void test_cipaddress_pton(const char* peer_ip, int family, const uint32_t (&ip)[4])
{
const int port = 4200;
Expand Down

0 comments on commit 4275d7b

Please sign in to comment.