Skip to content

Commit

Permalink
Use LambdaBridge to safely invoke functions in GLib Matter context (#…
Browse files Browse the repository at this point in the history
…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 <[email protected]>
  • Loading branch information
Alami-Amine and andreilitvin authored Sep 27, 2024
1 parent a6f44fd commit 1b1719a
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 49 deletions.
15 changes: 4 additions & 11 deletions src/platform/Linux/PlatformManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,14 +281,14 @@ void PlatformManagerImpl::_Shutdown()
}

#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();

Expand All @@ -300,26 +300,19 @@ CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(
// 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;
},
&invokeData, nullptr);

lock.lock();

invokeData.mDoneCond.wait(lock, [&invokeData]() { return invokeData.mDone; });

return invokeData.mFuncResult;
}
#endif // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP

Expand Down
30 changes: 23 additions & 7 deletions src/platform/Linux/PlatformManagerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

#pragma once

#include "lib/core/CHIPError.h"
#include <condition_variable>
#include <mutex>

Expand Down Expand Up @@ -69,7 +70,21 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
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)
Expand Down Expand Up @@ -102,9 +117,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener

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;
Expand All @@ -113,10 +126,13 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
/**
* @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.
Expand Down
14 changes: 4 additions & 10 deletions src/platform/NuttX/PlatformManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,14 +281,14 @@ void PlatformManagerImpl::_Shutdown()
}

#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();

Expand All @@ -300,15 +300,11 @@ CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(
// 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;
Expand All @@ -318,8 +314,6 @@ CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(
lock.lock();

invokeData.mDoneCond.wait(lock, [&invokeData]() { return invokeData.mDone; });

return invokeData.mFuncResult;
}
#endif // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP

Expand Down
29 changes: 22 additions & 7 deletions src/platform/NuttX/PlatformManagerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,21 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
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)
Expand Down Expand Up @@ -102,9 +116,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener

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;
Expand All @@ -113,10 +125,13 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
/**
* @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.
Expand Down
15 changes: 5 additions & 10 deletions src/platform/Tizen/PlatformManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,7 @@ namespace {

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;
Expand Down Expand Up @@ -144,18 +142,17 @@ void PlatformManagerImpl::_Shutdown()
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;
Expand All @@ -167,8 +164,6 @@ CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(

std::unique_lock<std::mutex> lock(invokeData.mDoneMutex);
invokeData.mDoneCond.wait(lock, [&invokeData]() { return invokeData.mDone; });

return invokeData.mFuncResult;
}

} // namespace DeviceLayer
Expand Down
25 changes: 21 additions & 4 deletions src/platform/Tizen/PlatformManagerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,21 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
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; }

Expand All @@ -87,10 +101,13 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
/**
* @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;
Expand Down

0 comments on commit 1b1719a

Please sign in to comment.