Revert TSAN workaround

scripts/tests/chiptest/tsan-linux-suppressions.txt

@@ -8,3 +8,7 @@
 # See https://github.com/project-chip/connectedhomeip/issues/14710 for
 # addressing this.
 called_from_lib:libglib
+
+# NOTE: This change will allow PR #45045 to be merged. In the future this whole
+#       suppression file will be removed anyway (when PR #23529 will get merged).
+race:g_main_context_dispatch

src/platform/Linux/PlatformManagerImpl.cpp

@@ -33,7 +33,8 @@
 #include <netinet/in.h>
 #include <unistd.h>
 
-#include <mutex>
+#include <condition_variable>
+#include <thread>
 
 #include <app-common/zap-generated/enums.h>
 #include <app-common/zap-generated/ids/Events.h>
@@ -56,12 +57,47 @@ 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
+
+#endif // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
 
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFI
 
@@ -173,10 +209,10 @@ CHIP_ERROR RunWiFiIPChangeListener()
     }
 
     GIOChannel * ch = g_io_channel_unix_new(sock);
-    g_io_add_watch_full(ch, G_PRIORITY_DEFAULT, G_IO_IN, WiFiIPChangeListener, nullptr, nullptr);
-
     g_io_channel_set_close_on_unref(ch, TRUE);
     g_io_channel_set_encoding(ch, nullptr, nullptr);
+
+    g_io_add_watch_full(ch, G_PRIORITY_DEFAULT, G_IO_IN, WiFiIPChangeListener, nullptr, nullptr);
     g_io_channel_unref(ch);
 
     return CHIP_NO_ERROR;
@@ -193,12 +229,9 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack()
     mGLibMainLoop       = g_main_loop_new(nullptr, FALSE);
     mGLibMainLoopThread = g_thread_new("gmain-matter", GLibMainLoopThread, mGLibMainLoop);
 
-    {
-        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);
-    }
+    CallbackIndirection startedInd([](void *) { return G_SOURCE_REMOVE; }, nullptr);
+    g_idle_add(G_SOURCE_FUNC(&CallbackIndirection::Callback), &startedInd);
+    startedInd.Wait();
 
 #endif
 
@@ -253,34 +286,7 @@ void PlatformManagerImpl::_Shutdown()
 #endif
 }
 
-#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
+#if CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP && CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE
 CHIP_ERROR PlatformManagerImpl::RunOnGLibMainLoopThread(GSourceFunc callback, void * userData, bool wait)
 {
 
@@ -290,19 +296,16 @@ 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(lock);
+        indirection.Wait();
         return CHIP_NO_ERROR;
     }
 
     g_main_context_invoke(context, callback, userData);
     return CHIP_NO_ERROR;
 }
-#endif // CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE
-
-#endif // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
+#endif
 
 } // namespace DeviceLayer
 } // namespace chip

src/platform/Linux/PlatformManagerImpl.h

@@ -23,9 +23,6 @@
 
 #pragma once
 
-#include <condition_variable>
-#include <mutex>
-
 #include <platform/PlatformManager.h>
 #include <platform/internal/GenericPlatformManagerImpl_POSIX.h>
 
@@ -96,35 +93,9 @@ 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 // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
+#endif
 };
 
 /**