Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use LambdaBridge to safely invoke functions in GLib Matter context #35777

Merged
merged 15 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading