Workaround for TSAN false positive reports with glib

TSAN thinks that memory accessed before the call to g_source_attach()
(which is internally used by e.g. g_main_context_invoke() and
g_idle_add()) needs to be guarded by a mutex, otherwise that's a race
condition between the thread that is creating shared data (the current
thread) and the glib main event loop thread. In fact such race condition
does not occur because g_source_attach() acts here as pthread_create() -
code is not executed on the other thread before the g_source_attach() is
called.

src/platform/Linux/PlatformManagerImpl.cpp

@@ -33,8 +33,7 @@
 #include <netinet/in.h>
 #include <unistd.h>
 
-#include <condition_variable>
-#include <thread>
+#include <mutex>
 
 #include <app-common/zap-generated/enums.h>
 #include <app-common/zap-generated/ids/Events.h>
@@ -57,47 +56,12 @@ PlatformManagerImpl PlatformManagerImpl::sInstance;
 namespace {
 
 #if CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
-
-class CallbackIndirection
-{
-public:
-    CallbackIndirection(GSourceFunc callback, void * userData) : mCallback(callback), mUserData(userData) {}
-
-    void Wait()
-    {
-        g_mutex_lock(&mDoneMtx);
-        while (!mDone)
-            g_cond_wait(&mDoneCond, &mDoneMtx);
-        g_mutex_unlock(&mDoneMtx);
-    }
-
-    static gboolean Callback(CallbackIndirection * self)
-    {
-        auto result = self->mCallback(self->mUserData);
-
-        g_mutex_lock(&self->mDoneMtx);
-        self->mDone = true;
-        g_mutex_unlock(&self->mDoneMtx);
-
-        g_cond_signal(&self->mDoneCond);
-        return result;
-    }
-
-private:
-    GMutex mDoneMtx{};
-    GCond mDoneCond{};
-    bool mDone = false;
-    GSourceFunc mCallback;
-    void * mUserData;
-};
-
 void * GLibMainLoopThread(void * loop)
 {
     g_main_loop_run(static_cast<GMainLoop *>(loop));
     return nullptr;
 }
-
-#endif // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
+#endif
 
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFI
 
@@ -229,9 +193,12 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack()
     mGLibMainLoop       = g_main_loop_new(nullptr, FALSE);
     mGLibMainLoopThread = g_thread_new("gmain-matter", GLibMainLoopThread, mGLibMainLoop);
 
-    CallbackIndirection startedInd([](void *) { return G_SOURCE_REMOVE; }, nullptr);
-    g_idle_add(G_SOURCE_FUNC(&CallbackIndirection::Callback), &startedInd);
-    startedInd.Wait();
+    {
+        std::unique_lock<std::mutex> lock(mGLibMainLoopCallbackIndirectionMutex);
+        CallbackIndirection startedInd([](void *) { return G_SOURCE_REMOVE; }, nullptr);
+        g_idle_add(G_SOURCE_FUNC(&CallbackIndirection::Callback), &startedInd);
+        startedInd.Wait(lock);
+    }
 
 #endif
 
@@ -286,7 +253,34 @@ void PlatformManagerImpl::_Shutdown()
 #endif
 }
 
-#if CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP && CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE
+#if CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
+
+void PlatformManagerImpl::CallbackIndirection::Wait(std::unique_lock<std::mutex> & lock)
+{
+    mDoneCond.wait(lock, [this]() { return mDone; });
+}
+
+gboolean PlatformManagerImpl::CallbackIndirection::Callback(CallbackIndirection * self)
+{
+    // We can not access "self" before acquiring the lock, because TSAN will complain that
+    // there is a race condition between the thread that created the object and the thread
+    // that is executing the callback.
+    std::unique_lock<std::mutex> lock(PlatformMgrImpl().mGLibMainLoopCallbackIndirectionMutex);
+
+    auto callback = self->mCallback;
+    auto userData = self->mUserData;
+
+    lock.unlock();
+    auto result = callback(userData);
+    lock.lock();
+
+    self->mDone = true;
+    self->mDoneCond.notify_all();
+
+    return result;
+}
+
+#if CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE
 CHIP_ERROR PlatformManagerImpl::RunOnGLibMainLoopThread(GSourceFunc callback, void * userData, bool wait)
 {
 
@@ -296,16 +290,19 @@ CHIP_ERROR PlatformManagerImpl::RunOnGLibMainLoopThread(GSourceFunc callback, vo
 
     if (wait)
     {
+        std::unique_lock<std::mutex> lock(mGLibMainLoopCallbackIndirectionMutex);
         CallbackIndirection indirection(callback, userData);
         g_main_context_invoke(context, G_SOURCE_FUNC(&CallbackIndirection::Callback), &indirection);
-        indirection.Wait();
+        indirection.Wait(lock);
         return CHIP_NO_ERROR;
     }
 
     g_main_context_invoke(context, callback, userData);
     return CHIP_NO_ERROR;
 }
-#endif
+#endif // CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE
+
+#endif // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
 
 } // namespace DeviceLayer
 } // namespace chip

src/platform/Linux/PlatformManagerImpl.h

@@ -23,6 +23,9 @@
 
 #pragma once
 
+#include <condition_variable>
+#include <mutex>
+
 #include <platform/PlatformManager.h>
 #include <platform/internal/GenericPlatformManagerImpl_POSIX.h>
 
@@ -93,9 +96,35 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
     static PlatformManagerImpl sInstance;
 
 #if CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
+
+    class CallbackIndirection
+    {
+    public:
+        CallbackIndirection(GSourceFunc callback, void * userData) : mCallback(callback), mUserData(userData) {}
+        void Wait(std::unique_lock<std::mutex> & lock);
+        static gboolean Callback(CallbackIndirection * self);
+
+    private:
+        GSourceFunc mCallback;
+        void * mUserData;
+        // Sync primitives to wait for the callback to be executed.
+        std::condition_variable mDoneCond;
+        bool mDone = false;
+    };
+
+    // XXX: Mutex for guarding access to glib main event loop callback indirection
+    //      synchronization primitives. This is a workaround to suppress TSAN warnings.
+    //      TSAN does not know that from the thread synchronization perspective the
+    //      g_source_attach() function should be treated as pthread_create(). Memory
+    //      access to shared data before the call to g_source_attach() without mutex
+    //      is not a race condition - the callback will not be executed on glib main
+    //      event loop thread before the call to g_source_attach().
+    std::mutex mGLibMainLoopCallbackIndirectionMutex;
+
     GMainLoop * mGLibMainLoop;
     GThread * mGLibMainLoopThread;
-#endif
+
+#endif // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
 };
 
 /**