From 00e6ecf01cd6eeb0b57c05cf8fbb2e0cf744e918 Mon Sep 17 00:00:00 2001
From: Zhao Zhili <zhilizhao@tencent.com>
Date: Thu, 15 Apr 2021 15:19:13 +0800
Subject: [PATCH] Fix GC stop handling

1. Protect m_bClosing by m_GCStopLock
2. Fix the issue that m_GCStopLock can be setup multiple times and
never be released. srt_startup()/srt_cleanup() meant to be called
only once, but it doesn't be used this way in FFmpeg/VLC/GStreamer.
---
 srtcore/api.cpp | 36 ++++++++++++++++--------------------
 srtcore/api.h   |  2 +-
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/srtcore/api.cpp b/srtcore/api.cpp
index 71e4a8772..122e52443 100644
--- a/srtcore/api.cpp
+++ b/srtcore/api.cpp
@@ -215,6 +215,8 @@ m_ClosedSockets()
    // 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_GCStopLock, "GCStop");
+   setupCond(m_GCStopCond, "GCStop");
    setupMutex(m_GlobControlLock, "GlobControl");
    setupMutex(m_IDLock, "ID");
    setupMutex(m_InitLock, "Init");
@@ -235,6 +237,16 @@ CUDTUnited::~CUDTUnited()
     releaseMutex(m_GlobControlLock);
     releaseMutex(m_IDLock);
     releaseMutex(m_InitLock);
+    // XXX There's some weird bug here causing this
+    // to hangup on Windows. This might be either something
+    // bigger, or some problem in pthread-win32. As this is
+    // the application cleanup section, this can be temporarily
+    // tolerated with simply exit the application without cleanup,
+    // counting on that the system will take care of it anyway.
+#ifndef _WIN32
+    releaseCond(m_GCStopCond);
+#endif
+    releaseMutex(m_GCStopLock);
 
     delete m_pCache;
 }
@@ -273,15 +285,6 @@ int CUDTUnited::startup()
 
    m_bClosing = false;
 
-   try
-   {
-       setupMutex(m_GCStopLock, "GCStop");
-       setupCond(m_GCStopCond, "GCStop");
-   }
-   catch (...)
-   {
-       return -1;
-   }
    if (!StartThread(m_GCThread, garbageCollect, this, "SRT:GC"))
       return -1;
 
@@ -310,7 +313,10 @@ int CUDTUnited::cleanup()
    if (!m_bGCStatus)
       return 0;
 
-   m_bClosing = true;
+   {
+       UniqueLock gclock(m_GCStopLock);
+       m_bClosing = true;
+   }
    // NOTE: we can do relaxed signaling here because
    // waiting on m_GCStopCond has a 1-second timeout,
    // after which the m_bClosing flag is cheched, which
@@ -319,16 +325,6 @@ int CUDTUnited::cleanup()
    CSync::signal_relaxed(m_GCStopCond);
    m_GCThread.join();
 
-   // XXX There's some weird bug here causing this
-   // to hangup on Windows. This might be either something
-   // bigger, or some problem in pthread-win32. As this is
-   // the application cleanup section, this can be temporarily
-   // tolerated with simply exit the application without cleanup,
-   // counting on that the system will take care of it anyway.
-#ifndef _WIN32
-   releaseCond(m_GCStopCond);
-#endif
-
    m_bGCStatus = false;
 
    // Global destruction code
diff --git a/srtcore/api.h b/srtcore/api.h
index 604f14a99..6bb3acdb3 100644
--- a/srtcore/api.h
+++ b/srtcore/api.h
@@ -414,7 +414,7 @@ friend class CRendezvousQueue;
    CCache<CInfoBlock>* m_pCache;			// UDT network information cache
 
 private:
-   volatile bool m_bClosing;
+   bool m_bClosing;
    srt::sync::Mutex m_GCStopLock;
    srt::sync::Condition m_GCStopCond;