Use LambdaBridge to safely invoke functions in GLib Matter context (#35777)
* using LambdaBridge to cast function pointer
* adding support for Lambda that returns a CHIP_ERROR to LambdaBridge
* pass an out pointer to capture function return result
* Tizen Platform: Using LambdaBridge to safely invoke functions in GLib Matter context
* Update to not modify lambdabridge
* Potentiall cleaner code
* Update code to make it compile
* Update tizen code too
* Tizen platform fix
* Duplicating Changes to NuttX platform
* Tizen Platform Fix
---------
Co-authored-by: Andrei Litvin <andreilitvin@google.com>
diff --git a/src/platform/Linux/PlatformManagerImpl.cpp b/src/platform/Linux/PlatformManagerImpl.cpp
index 68f3d58..dba3cb4 100644
--- a/src/platform/Linux/PlatformManagerImpl.cpp
+++ b/src/platform/Linux/PlatformManagerImpl.cpp
@@ -281,14 +281,14 @@
}
#if CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
-CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(void *), void * userData)
+void PlatformManagerImpl::_GLibMatterContextInvokeSync(LambdaBridge && bridge)
{
// Because of TSAN false positives, we need to use a mutex to synchronize access to all members of
// the GLibMatterContextInvokeData object (including constructor and destructor). This is a temporary
// workaround until TSAN-enabled GLib will be used in our CI.
std::unique_lock<std::mutex> lock(mGLibMainLoopCallbackIndirectionMutex);
- GLibMatterContextInvokeData invokeData{ func, userData };
+ GLibMatterContextInvokeData invokeData{ std::move(bridge) };
lock.unlock();
@@ -300,15 +300,11 @@
// XXX: Temporary workaround for TSAN false positives.
std::unique_lock<std::mutex> lock_(PlatformMgrImpl().mGLibMainLoopCallbackIndirectionMutex);
- auto mFunc = data->mFunc;
- auto mUserData = data->mFuncUserData;
-
lock_.unlock();
- auto result = mFunc(mUserData);
+ data->bridge();
lock_.lock();
- data->mDone = true;
- data->mFuncResult = result;
+ data->mDone = true;
data->mDoneCond.notify_one();
return G_SOURCE_REMOVE;
@@ -316,10 +312,7 @@
&invokeData, nullptr);
lock.lock();
-
invokeData.mDoneCond.wait(lock, [&invokeData]() { return invokeData.mDone; });
-
- return invokeData.mFuncResult;
}
#endif // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
diff --git a/src/platform/Linux/PlatformManagerImpl.h b/src/platform/Linux/PlatformManagerImpl.h
index 5669ce5..d50687b 100644
--- a/src/platform/Linux/PlatformManagerImpl.h
+++ b/src/platform/Linux/PlatformManagerImpl.h
@@ -23,6 +23,7 @@
#pragma once
+#include "lib/core/CHIPError.h"
#include <condition_variable>
#include <mutex>
@@ -69,7 +70,21 @@
template <typename T>
CHIP_ERROR GLibMatterContextInvokeSync(CHIP_ERROR (*func)(T *), T * userData)
{
- return _GLibMatterContextInvokeSync((CHIP_ERROR(*)(void *)) func, (void *) userData);
+ struct
+ {
+ CHIP_ERROR returnValue = CHIP_NO_ERROR;
+ CHIP_ERROR (*functionToCall)(T *);
+ T * userData;
+ } context;
+
+ context.functionToCall = func;
+ context.userData = userData;
+
+ LambdaBridge bridge;
+ bridge.Initialize([&context]() { context.returnValue = context.functionToCall(context.userData); });
+
+ _GLibMatterContextInvokeSync(std::move(bridge));
+ return context.returnValue;
}
unsigned int GLibMatterContextAttachSource(GSource * source)
@@ -102,9 +117,7 @@
struct GLibMatterContextInvokeData
{
- CHIP_ERROR (*mFunc)(void *);
- void * mFuncUserData;
- CHIP_ERROR mFuncResult;
+ LambdaBridge bridge;
// Sync primitives to wait for the function to be executed
std::condition_variable mDoneCond;
bool mDone = false;
@@ -113,10 +126,13 @@
/**
* @brief Invoke a function on the Matter GLib context.
*
- * @note This function does not provide type safety for the user data. Please,
- * use the GLibMatterContextInvokeSync() template function instead.
+ * @param[in] bridge a LambdaBridge object that holds the lambda to be invoked within the GLib context.
+ *
+ * @note This function moves the LambdaBridge into the GLib context for invocation.
+ * The LambdaBridge is created and initialised in GLibMatterContextInvokeSync().
+ * use the GLibMatterContextInvokeSync() template function instead of this one.
*/
- CHIP_ERROR _GLibMatterContextInvokeSync(CHIP_ERROR (*func)(void *), void * userData);
+ void _GLibMatterContextInvokeSync(LambdaBridge && bridge);
// XXX: Mutex for guarding access to glib main event loop callback indirection
// synchronization primitives. This is a workaround to suppress TSAN warnings.
diff --git a/src/platform/NuttX/PlatformManagerImpl.cpp b/src/platform/NuttX/PlatformManagerImpl.cpp
index d590f58..a44f084 100644
--- a/src/platform/NuttX/PlatformManagerImpl.cpp
+++ b/src/platform/NuttX/PlatformManagerImpl.cpp
@@ -281,14 +281,14 @@
}
#if CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
-CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(void *), void * userData)
+void PlatformManagerImpl::_GLibMatterContextInvokeSync(LambdaBridge && bridge)
{
// Because of TSAN false positives, we need to use a mutex to synchronize access to all members of
// the GLibMatterContextInvokeData object (including constructor and destructor). This is a temporary
// workaround until TSAN-enabled GLib will be used in our CI.
std::unique_lock<std::mutex> lock(mGLibMainLoopCallbackIndirectionMutex);
- GLibMatterContextInvokeData invokeData{ func, userData };
+ GLibMatterContextInvokeData invokeData{ std::move(bridge) };
lock.unlock();
@@ -300,15 +300,11 @@
// XXX: Temporary workaround for TSAN false positives.
std::unique_lock<std::mutex> lock_(PlatformMgrImpl().mGLibMainLoopCallbackIndirectionMutex);
- auto mFunc = data->mFunc;
- auto mUserData = data->mFuncUserData;
-
lock_.unlock();
- auto result = mFunc(mUserData);
+ data->bridge();
lock_.lock();
- data->mDone = true;
- data->mFuncResult = result;
+ data->mDone = true;
data->mDoneCond.notify_one();
return G_SOURCE_REMOVE;
@@ -318,8 +314,6 @@
lock.lock();
invokeData.mDoneCond.wait(lock, [&invokeData]() { return invokeData.mDone; });
-
- return invokeData.mFuncResult;
}
#endif // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
diff --git a/src/platform/NuttX/PlatformManagerImpl.h b/src/platform/NuttX/PlatformManagerImpl.h
index 5669ce5..f911ccb 100644
--- a/src/platform/NuttX/PlatformManagerImpl.h
+++ b/src/platform/NuttX/PlatformManagerImpl.h
@@ -69,7 +69,21 @@
template <typename T>
CHIP_ERROR GLibMatterContextInvokeSync(CHIP_ERROR (*func)(T *), T * userData)
{
- return _GLibMatterContextInvokeSync((CHIP_ERROR(*)(void *)) func, (void *) userData);
+ struct
+ {
+ CHIP_ERROR returnValue = CHIP_NO_ERROR;
+ CHIP_ERROR (*functionToCall)(T *);
+ T * userData;
+ } context;
+
+ context.functionToCall = func;
+ context.userData = userData;
+
+ LambdaBridge bridge;
+ bridge.Initialize([&context]() { context.returnValue = context.functionToCall(context.userData); });
+
+ _GLibMatterContextInvokeSync(std::move(bridge));
+ return context.returnValue;
}
unsigned int GLibMatterContextAttachSource(GSource * source)
@@ -102,9 +116,7 @@
struct GLibMatterContextInvokeData
{
- CHIP_ERROR (*mFunc)(void *);
- void * mFuncUserData;
- CHIP_ERROR mFuncResult;
+ LambdaBridge bridge;
// Sync primitives to wait for the function to be executed
std::condition_variable mDoneCond;
bool mDone = false;
@@ -113,10 +125,13 @@
/**
* @brief Invoke a function on the Matter GLib context.
*
- * @note This function does not provide type safety for the user data. Please,
- * use the GLibMatterContextInvokeSync() template function instead.
+ * @param[in] bridge a LambdaBridge object that holds the lambda to be invoked within the GLib context.
+ *
+ * @note This function moves the LambdaBridge into the GLib context for invocation.
+ * The LambdaBridge is created and initialised in GLibMatterContextInvokeSync().
+ * use the GLibMatterContextInvokeSync() template function instead of this one.
*/
- CHIP_ERROR _GLibMatterContextInvokeSync(CHIP_ERROR (*func)(void *), void * userData);
+ void _GLibMatterContextInvokeSync(LambdaBridge && bridge);
// XXX: Mutex for guarding access to glib main event loop callback indirection
// synchronization primitives. This is a workaround to suppress TSAN warnings.
diff --git a/src/platform/Tizen/PlatformManagerImpl.cpp b/src/platform/Tizen/PlatformManagerImpl.cpp
index 58e1e01..91cb91b 100644
--- a/src/platform/Tizen/PlatformManagerImpl.cpp
+++ b/src/platform/Tizen/PlatformManagerImpl.cpp
@@ -56,9 +56,7 @@
struct GLibMatterContextInvokeData
{
- CHIP_ERROR (*mFunc)(void *);
- void * mFuncUserData;
- CHIP_ERROR mFuncResult;
+ LambdaBridge bridge;
// Sync primitives to wait for the function to be executed
std::mutex mDoneMutex;
std::condition_variable mDoneCond;
@@ -144,18 +142,17 @@
mGLibMainLoop = nullptr;
}
-CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(void *), void * userData)
+void PlatformManagerImpl::_GLibMatterContextInvokeSync(LambdaBridge && bridge)
{
- GLibMatterContextInvokeData invokeData{ func, userData };
+ GLibMatterContextInvokeData invokeData{ std::move(bridge) };
g_main_context_invoke_full(
g_main_loop_get_context(mGLibMainLoop), G_PRIORITY_HIGH_IDLE,
[](void * userData_) {
auto * data = reinterpret_cast<GLibMatterContextInvokeData *>(userData_);
VerifyOrExit(g_main_context_get_thread_default() != nullptr,
- ChipLogError(DeviceLayer, "GLib thread default main context is not set");
- data->mFuncResult = CHIP_ERROR_INCORRECT_STATE);
- data->mFuncResult = data->mFunc(data->mFuncUserData);
+ ChipLogError(DeviceLayer, "GLib thread default main context is not set"));
+ data->bridge();
exit:
data->mDoneMutex.lock();
data->mDone = true;
@@ -167,8 +164,6 @@
std::unique_lock<std::mutex> lock(invokeData.mDoneMutex);
invokeData.mDoneCond.wait(lock, [&invokeData]() { return invokeData.mDone; });
-
- return invokeData.mFuncResult;
}
} // namespace DeviceLayer
diff --git a/src/platform/Tizen/PlatformManagerImpl.h b/src/platform/Tizen/PlatformManagerImpl.h
index 8a3d0e4..ebb61aa 100644
--- a/src/platform/Tizen/PlatformManagerImpl.h
+++ b/src/platform/Tizen/PlatformManagerImpl.h
@@ -64,7 +64,21 @@
template <typename T>
CHIP_ERROR GLibMatterContextInvokeSync(CHIP_ERROR (*func)(T *), T * userData)
{
- return _GLibMatterContextInvokeSync((CHIP_ERROR(*)(void *)) func, (void *) userData);
+ struct
+ {
+ CHIP_ERROR returnValue = CHIP_NO_ERROR;
+ CHIP_ERROR (*functionToCall)(T *);
+ T * userData;
+ } context;
+
+ context.functionToCall = func;
+ context.userData = userData;
+
+ LambdaBridge bridge;
+ bridge.Initialize([&context]() { context.returnValue = context.functionToCall(context.userData); });
+
+ _GLibMatterContextInvokeSync(std::move(bridge));
+ return context.returnValue;
}
System::Clock::Timestamp GetStartTime() { return mStartTime; }
@@ -87,10 +101,13 @@
/**
* @brief Invoke a function on the Matter GLib context.
*
- * @note This function does not provide type safety for the user data. Please,
- * use the GLibMatterContextInvokeSync() template function instead.
+ * @param[in] bridge a LambdaBridge object that holds the lambda to be invoked within the GLib context.
+ *
+ * @note This function moves the LambdaBridge into the GLib context for invocation.
+ * The LambdaBridge is created and initialised in GLibMatterContextInvokeSync().
+ * use the GLibMatterContextInvokeSync() template function instead of this one.
*/
- CHIP_ERROR _GLibMatterContextInvokeSync(CHIP_ERROR (*func)(void *), void * userData);
+ void _GLibMatterContextInvokeSync(LambdaBridge && bridge);
GMainLoop * mGLibMainLoop;
GThread * mGLibMainLoopThread;
