[maint] Reworked C++03 solution for thread-local error

apps/srt-tunnel.cpp

@@ -470,7 +470,7 @@ class SrtMedium: public Medium
 
     using Medium::Error;
 
-    static void Error(UDT::ERRORINFO& ri, const string& text)
+    static void Error(const UDT::ERRORINFO& ri, const string& text)
     {
         throw TransmissionError("ERROR: " + text + ": " + ri.getErrorMessage());
     }

srtcore/api.cpp

@@ -1862,7 +1862,7 @@ int srt::CUDTUnited::groupConnect(CUDTGroup* pg, SRT_SOCKGROUPCONFIG* targets, i
         if (estat == -1)
         {
 #if ENABLE_LOGGING
-            CUDTException& x = CUDT::getlasterror();
+            const CUDTException& x = CUDT::getlasterror();
             if (x.getErrorCode() != SRT_EPOLLEMPTY)
             {
                 LOGC(aclog.Error,
@@ -4445,7 +4445,7 @@ int srt::CUDT::epoll_release(const int eid)
     }
 }
 
-srt::CUDTException& srt::CUDT::getlasterror()
+const srt::CUDTException& srt::CUDT::getlasterror()
 {
     return GetThreadLocalError();
 }
@@ -4821,7 +4821,7 @@ int epoll_release(int eid)
     return srt::CUDT::epoll_release(eid);
 }
 
-ERRORINFO& getlasterror()
+const ERRORINFO& getlasterror()
 {
     return srt::CUDT::getlasterror();
 }

srtcore/core.h

@@ -370,7 +370,7 @@ class CUDT
     static int epoll_uwait(const int eid, SRT_EPOLL_EVENT* fdsSet, int fdsSize, int64_t msTimeOut);
     static int32_t epoll_set(const int eid, int32_t flags);
     static int epoll_release(const int eid);
-    static CUDTException& getlasterror();
+    static const CUDTException& getlasterror();
     static int bstats(SRTSOCKET u, CBytePerfMon* perf, bool clear = true, bool instantaneous = false);
 #if ENABLE_BONDING
     static int groupsockbstats(SRTSOCKET u, CBytePerfMon* perf, bool clear = true);

srtcore/srt_c_api.cpp

@@ -22,6 +22,7 @@ written by
 #include "packet.h"
 #include "core.h"
 #include "utilities.h"
+#include "sync.h"
 
 using namespace std;
 using namespace srt;
@@ -264,7 +265,7 @@ const char* srt_strerror(int code, int /*err ignored*/)
 
 void srt_clearlasterror()
 {
-    UDT::getlasterror().clear();
+    srt::sync::ClearThreadLocalError();
 }
 
 int srt_bstats(SRTSOCKET u, SRT_TRACEBSTATS * perf, int clear) { return CUDT::bstats(u, perf, 0!=  clear); }

srtcore/sync.cpp

@@ -459,3 +459,131 @@ int srt::sync::SharedMutex::getReaderCount() const
 }
 #endif // C++17 for shared_mutex
 
+//
+//  Thread local error accessors.
+//
+
+namespace srt
+{
+namespace sync
+{
+
+#if HAVE_FULL_CXX11
+
+// NOTE: This version is used always with C++11, even if you require POSIX
+// version of the sync module. OTOH if you don't use C++11, then POSIX
+// version is your only option anyway.
+
+static CUDTException& AccessThreadLocalObject()
+{
+    static thread_local CUDTException thread_local_error;
+    return thread_local_error;
+}
+
+const CUDTException& GetThreadLocalError() { return AccessThreadLocalObject(); }
+void SetThreadLocalError(const CUDTException& e) { AccessThreadLocalObject() = e; }
+void ClearThreadLocalError() { AccessThreadLocalObject().clear(); }
+
+#else
+
+// Shouldn't be an issue, but just in case
+#ifdef ENABLE_STDCXX_SYNC
+#error With no C++11 available, ENABLE_STDCXX_SYNC shall not be defined
+#endif
+
+// Elaborate version if compiling in C++98 mode. Instead
+// of thread_local we use CThreadLocal class that uses
+// pthread_getspecific to access the thread-local object.
+
+// IMPORTANT!!! The instance of this class is only allowed to be declared
+// as global (including local static).
+template<class TargetObject>
+class CThreadLocal
+{
+public:
+    CThreadLocal()
+    {
+        // TODO: Would be nice to make sure that this is done
+        // before anything has started.
+        pthread_key_create(&m_ThreadSpecKey, FwdDelete);
+    }
+
+    ~CThreadLocal()
+    {
+        // This would be done anyway, but we speed things up here
+        // and prevent this action from happening too late.
+        void* po = pthread_getspecific(m_ThreadSpecKey);
+        if (po)
+            FwdDelete(po);
+        pthread_setspecific(m_ThreadSpecKey, NULL);
+        pthread_key_delete(m_ThreadSpecKey);
+    }
+
+    TargetObject* get()
+    {
+        void* po = pthread_getspecific(m_ThreadSpecKey);
+        if (!po)
+        {
+            // DO NOT throw exception here, while the key is allowed
+            // to be NULL. A fallback for NULL object will be organized.
+            po = new(std::nothrow) TargetObject();
+            pthread_setspecific(m_ThreadSpecKey, po);
+        }
+        return (TargetObject*)po;
+    }
+
+    void operator=(const TargetObject& source)
+    {
+        TargetObject* target = get();
+        if (target)
+            *target = source;
+    }
+
+    static void FwdDelete(void* e)
+    {
+        delete (TargetObject*)e;
+    }
+
+private:
+    pthread_key_t m_ThreadSpecKey;
+};
+
+static CThreadLocal<CUDTException>& AccessThreadLocalObject()
+{
+    // NOTE: C++98 doesn't guarantee safe initialization of the static local
+    // objects, but compilers do, at least those released after 2005 year,
+    // regardless what standard is set for compiling.
+    static CThreadLocal<CUDTException> thread_local_error;
+    return thread_local_error;
+}
+
+const CUDTException& GetThreadLocalError()
+{
+    // In the posix version we allocate the objects dynamically, hence
+    // there exists a theoretical possibility for that to fail. The fallback
+    // for that case is to return the memory allocation error always.
+    static CUDTException resident_alloc_error (MJ_SYSTEMRES, MN_MEMORY);
+    CUDTException* curx = AccessThreadLocalObject().get();
+    if (!curx)
+        return resident_alloc_error;
+    return *curx;
+}
+
+void SetThreadLocalError(const CUDTException& e)
+{
+    AccessThreadLocalObject() = e;
+}
+
+void ClearThreadLocalError()
+{
+    CUDTException* curx = AccessThreadLocalObject().get();
+    if (curx)
+        curx->clear(); // ignore otherwise
+}
+
+#endif
+
+}
+}
+
+

srtcore/sync.h

@@ -1059,13 +1059,9 @@ bool StartThread(CThread& th, void* (*f) (void*), void* args, const std::string&
 //
 ////////////////////////////////////////////////////////////////////////////////
 
-/// Set thread local error
-/// @param e new CUDTException
 void SetThreadLocalError(const CUDTException& e);
-
-/// Get thread local error
-/// @returns CUDTException pointer
-CUDTException& GetThreadLocalError();
+const CUDTException& GetThreadLocalError();
+void ClearThreadLocalError();
 
 ////////////////////////////////////////////////////////////////////////////////
 //

srtcore/sync_cxx11.cpp

@@ -104,23 +104,3 @@ void srt::sync::Condition::notify_all()
     m_cv.notify_all();
 }
 
-////////////////////////////////////////////////////////////////////////////////
-//
-// CThreadError class - thread local storage error wrapper
-//
-////////////////////////////////////////////////////////////////////////////////
-
-// Threal local error will be used by CUDTUnited
-// with a static scope, therefore static thread_local
-static thread_local srt::CUDTException s_thErr;
-
-void srt::sync::SetThreadLocalError(const srt::CUDTException& e)
-{
-    s_thErr = e;
-}
-
-srt::CUDTException& srt::sync::GetThreadLocalError()
-{
-    return s_thErr;
-}
-

srtcore/sync_posix.cpp

@@ -464,120 +464,3 @@ void srt::sync::CThread::create(void *(*start_routine) (void *), void *arg)
     m_pid = getpid();
 }
 
-
-////////////////////////////////////////////////////////////////////////////////
-//
-// CThreadError class - thread local storage error wrapper
-//
-////////////////////////////////////////////////////////////////////////////////
-namespace srt {
-namespace sync {
-
-class CThreadError
-{
-public:
-    CThreadError()
-    {
-        pthread_key_create(&m_ThreadSpecKey, ThreadSpecKeyDestroy);
-
-        // This is a global object and as such it should be called in the
-        // main application thread or at worst in the thread that has first
-        // run `srt_startup()` function and so requested the SRT library to
-        // be dynamically linked. Most probably in this very thread the API
-        // errors will be reported, so preallocate the ThreadLocalSpecific
-        // object for this error description.
-
-        // This allows std::bac_alloc to crash the program during
-        // the initialization of the SRT library (likely it would be
-        // during the DL constructor, still way before any chance of
-        // doing any operations here). This will prevent SRT from running
-        // into trouble while trying to operate.
-        CUDTException* ne = new CUDTException();
-        pthread_setspecific(m_ThreadSpecKey, ne);
-    }
-
-    ~CThreadError()
-    {
-        // Likely all objects should be deleted in all
-        // threads that have exited, but std::this_thread didn't exit
-        // yet :).
-        ThreadSpecKeyDestroy(pthread_getspecific(m_ThreadSpecKey));
-        pthread_key_delete(m_ThreadSpecKey);
-    }
-
-    void set(const CUDTException& e)
-    {
-        CUDTException* cur = get();
-        // If this returns NULL, it means that there was an unexpected
-        // memory allocation error. Simply ignore this request if so
-        // happened, and then when trying to get the error description
-        // the application will always get the memory allocation error.
-
-        // There's no point in doing anything else here; lack of memory
-        // must be prepared for prematurely, and that was already done.
-        if (!cur)
-            return;
-
-        *cur = e;
-    }
-
-    /*[[nullable]]*/ CUDTException* get()
-    {
-        if (!pthread_getspecific(m_ThreadSpecKey))
-        {
-            // This time if this can't be done due to memory allocation
-            // problems, just allow this value to be NULL, which during
-            // getting the error description will redirect to a memory
-            // allocation error.
-
-            // It would be nice to somehow ensure that this object is
-            // created in every thread of the application using SRT, but
-            // POSIX thread API doesn't contain any possibility to have
-            // a creation callback that would apply to every thread in
-            // the application (as it is for C++11 thread_local storage).
-            CUDTException* ne = new(std::nothrow) CUDTException();
-            pthread_setspecific(m_ThreadSpecKey, ne);
-            return ne;
-        }
-        return (CUDTException*)pthread_getspecific(m_ThreadSpecKey);
-    }
-
-    static void ThreadSpecKeyDestroy(void* e)
-    {
-        delete (CUDTException*)e;
-    }
-
-private:
-    pthread_key_t m_ThreadSpecKey;
-};
-
-// Threal local error will be used by CUDTUnited
-// that has a static scope
-
-// This static makes this object file-private access so that
-// the access is granted only for the accessor functions.
-static CThreadError s_thErr;
-
-void SetThreadLocalError(const CUDTException& e)
-{
-    s_thErr.set(e);
-}
-
-CUDTException& GetThreadLocalError()
-{
-    // In POSIX version we take into account the possibility
-    // of having an allocation error here. Therefore we need to
-    // allow this value to return NULL and have some fallback
-    // for that case. The dynamic memory allocation failure should
-    // be the only case as to why it is unable to get the pointer
-    // to the error description.
-    static CUDTException resident_alloc_error (MJ_SYSTEMRES, MN_MEMORY);
-    CUDTException* curx = s_thErr.get();
-    if (!curx)
-        return resident_alloc_error;
-    return *curx;
-}
-
-} // namespace sync
-} // namespace srt
-

srtcore/udt.h

@@ -212,7 +212,7 @@ SRT_API int epoll_wait2(int eid, SRTSOCKET* readfds, int* rnum, SRTSOCKET* write
                         SYSSOCKET* lrfds = NULL, int* lrnum = NULL, SYSSOCKET* lwfds = NULL, int* lwnum = NULL);
 SRT_API int epoll_uwait(const int eid, SRT_EPOLL_EVENT* fdsSet, int fdsSize, int64_t msTimeOut);
 SRT_API int epoll_release(int eid);
-SRT_API ERRORINFO& getlasterror();
+SRT_API const ERRORINFO& getlasterror();
 SRT_API int getlasterror_code();
 SRT_API const char* getlasterror_desc();
 SRT_API int bstats(SRTSOCKET u, SRT_TRACEBSTATS* perf, bool clear = true);