Skip to content

Commit 1b1719a

Browse files
Alami-Amineandreilitvin
Alami-Amine
and
andreilitvin
authored
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>
1 parent a6f44fd commit 1b1719a

6 files changed

+79
-49
lines changed

‎src/platform/Linux/PlatformManagerImpl.cpp

+4-11
Original file line numberDiff line numberDiff line change
@@ -281,14 +281,14 @@ void PlatformManagerImpl::_Shutdown()
281281
}
282282

283283
#if CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
284-
CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(void *), void * userData)
284+
void PlatformManagerImpl::_GLibMatterContextInvokeSync(LambdaBridge && bridge)
285285
{
286286
// Because of TSAN false positives, we need to use a mutex to synchronize access to all members of
287287
// the GLibMatterContextInvokeData object (including constructor and destructor). This is a temporary
288288
// workaround until TSAN-enabled GLib will be used in our CI.
289289
std::unique_lock<std::mutex> lock(mGLibMainLoopCallbackIndirectionMutex);
290290

291-
GLibMatterContextInvokeData invokeData{ func, userData };
291+
GLibMatterContextInvokeData invokeData{ std::move(bridge) };
292292

293293
lock.unlock();
294294

@@ -300,26 +300,19 @@ CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(
300300
// XXX: Temporary workaround for TSAN false positives.
301301
std::unique_lock<std::mutex> lock_(PlatformMgrImpl().mGLibMainLoopCallbackIndirectionMutex);
302302

303-
auto mFunc = data->mFunc;
304-
auto mUserData = data->mFuncUserData;
305-
306303
lock_.unlock();
307-
auto result = mFunc(mUserData);
304+
data->bridge();
308305
lock_.lock();
309306

310-
data->mDone = true;
311-
data->mFuncResult = result;
307+
data->mDone = true;
312308
data->mDoneCond.notify_one();
313309

314310
return G_SOURCE_REMOVE;
315311
},
316312
&invokeData, nullptr);
317313

318314
lock.lock();
319-
320315
invokeData.mDoneCond.wait(lock, [&invokeData]() { return invokeData.mDone; });
321-
322-
return invokeData.mFuncResult;
323316
}
324317
#endif // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
325318

‎src/platform/Linux/PlatformManagerImpl.h

+23-7
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
#pragma once
2525

26+
#include "lib/core/CHIPError.h"
2627
#include <condition_variable>
2728
#include <mutex>
2829

@@ -69,7 +70,21 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
6970
template <typename T>
7071
CHIP_ERROR GLibMatterContextInvokeSync(CHIP_ERROR (*func)(T *), T * userData)
7172
{
72-
return _GLibMatterContextInvokeSync((CHIP_ERROR(*)(void *)) func, (void *) userData);
73+
struct
74+
{
75+
CHIP_ERROR returnValue = CHIP_NO_ERROR;
76+
CHIP_ERROR (*functionToCall)(T *);
77+
T * userData;
78+
} context;
79+
80+
context.functionToCall = func;
81+
context.userData = userData;
82+
83+
LambdaBridge bridge;
84+
bridge.Initialize([&context]() { context.returnValue = context.functionToCall(context.userData); });
85+
86+
_GLibMatterContextInvokeSync(std::move(bridge));
87+
return context.returnValue;
7388
}
7489

7590
unsigned int GLibMatterContextAttachSource(GSource * source)
@@ -102,9 +117,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
102117

103118
struct GLibMatterContextInvokeData
104119
{
105-
CHIP_ERROR (*mFunc)(void *);
106-
void * mFuncUserData;
107-
CHIP_ERROR mFuncResult;
120+
LambdaBridge bridge;
108121
// Sync primitives to wait for the function to be executed
109122
std::condition_variable mDoneCond;
110123
bool mDone = false;
@@ -113,10 +126,13 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
113126
/**
114127
* @brief Invoke a function on the Matter GLib context.
115128
*
116-
* @note This function does not provide type safety for the user data. Please,
117-
* use the GLibMatterContextInvokeSync() template function instead.
129+
* @param[in] bridge a LambdaBridge object that holds the lambda to be invoked within the GLib context.
130+
*
131+
* @note This function moves the LambdaBridge into the GLib context for invocation.
132+
* The LambdaBridge is created and initialised in GLibMatterContextInvokeSync().
133+
* use the GLibMatterContextInvokeSync() template function instead of this one.
118134
*/
119-
CHIP_ERROR _GLibMatterContextInvokeSync(CHIP_ERROR (*func)(void *), void * userData);
135+
void _GLibMatterContextInvokeSync(LambdaBridge && bridge);
120136

121137
// XXX: Mutex for guarding access to glib main event loop callback indirection
122138
// synchronization primitives. This is a workaround to suppress TSAN warnings.

‎src/platform/NuttX/PlatformManagerImpl.cpp

+4-10
Original file line numberDiff line numberDiff line change
@@ -281,14 +281,14 @@ void PlatformManagerImpl::_Shutdown()
281281
}
282282

283283
#if CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
284-
CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(void *), void * userData)
284+
void PlatformManagerImpl::_GLibMatterContextInvokeSync(LambdaBridge && bridge)
285285
{
286286
// Because of TSAN false positives, we need to use a mutex to synchronize access to all members of
287287
// the GLibMatterContextInvokeData object (including constructor and destructor). This is a temporary
288288
// workaround until TSAN-enabled GLib will be used in our CI.
289289
std::unique_lock<std::mutex> lock(mGLibMainLoopCallbackIndirectionMutex);
290290

291-
GLibMatterContextInvokeData invokeData{ func, userData };
291+
GLibMatterContextInvokeData invokeData{ std::move(bridge) };
292292

293293
lock.unlock();
294294

@@ -300,15 +300,11 @@ CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(
300300
// XXX: Temporary workaround for TSAN false positives.
301301
std::unique_lock<std::mutex> lock_(PlatformMgrImpl().mGLibMainLoopCallbackIndirectionMutex);
302302

303-
auto mFunc = data->mFunc;
304-
auto mUserData = data->mFuncUserData;
305-
306303
lock_.unlock();
307-
auto result = mFunc(mUserData);
304+
data->bridge();
308305
lock_.lock();
309306

310-
data->mDone = true;
311-
data->mFuncResult = result;
307+
data->mDone = true;
312308
data->mDoneCond.notify_one();
313309

314310
return G_SOURCE_REMOVE;
@@ -318,8 +314,6 @@ CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(
318314
lock.lock();
319315

320316
invokeData.mDoneCond.wait(lock, [&invokeData]() { return invokeData.mDone; });
321-
322-
return invokeData.mFuncResult;
323317
}
324318
#endif // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP
325319

‎src/platform/NuttX/PlatformManagerImpl.h

+22-7
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,21 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
6969
template <typename T>
7070
CHIP_ERROR GLibMatterContextInvokeSync(CHIP_ERROR (*func)(T *), T * userData)
7171
{
72-
return _GLibMatterContextInvokeSync((CHIP_ERROR(*)(void *)) func, (void *) userData);
72+
struct
73+
{
74+
CHIP_ERROR returnValue = CHIP_NO_ERROR;
75+
CHIP_ERROR (*functionToCall)(T *);
76+
T * userData;
77+
} context;
78+
79+
context.functionToCall = func;
80+
context.userData = userData;
81+
82+
LambdaBridge bridge;
83+
bridge.Initialize([&context]() { context.returnValue = context.functionToCall(context.userData); });
84+
85+
_GLibMatterContextInvokeSync(std::move(bridge));
86+
return context.returnValue;
7387
}
7488

7589
unsigned int GLibMatterContextAttachSource(GSource * source)
@@ -102,9 +116,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
102116

103117
struct GLibMatterContextInvokeData
104118
{
105-
CHIP_ERROR (*mFunc)(void *);
106-
void * mFuncUserData;
107-
CHIP_ERROR mFuncResult;
119+
LambdaBridge bridge;
108120
// Sync primitives to wait for the function to be executed
109121
std::condition_variable mDoneCond;
110122
bool mDone = false;
@@ -113,10 +125,13 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
113125
/**
114126
* @brief Invoke a function on the Matter GLib context.
115127
*
116-
* @note This function does not provide type safety for the user data. Please,
117-
* use the GLibMatterContextInvokeSync() template function instead.
128+
* @param[in] bridge a LambdaBridge object that holds the lambda to be invoked within the GLib context.
129+
*
130+
* @note This function moves the LambdaBridge into the GLib context for invocation.
131+
* The LambdaBridge is created and initialised in GLibMatterContextInvokeSync().
132+
* use the GLibMatterContextInvokeSync() template function instead of this one.
118133
*/
119-
CHIP_ERROR _GLibMatterContextInvokeSync(CHIP_ERROR (*func)(void *), void * userData);
134+
void _GLibMatterContextInvokeSync(LambdaBridge && bridge);
120135

121136
// XXX: Mutex for guarding access to glib main event loop callback indirection
122137
// synchronization primitives. This is a workaround to suppress TSAN warnings.

‎src/platform/Tizen/PlatformManagerImpl.cpp

+5-10
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,7 @@ namespace {
5656

5757
struct GLibMatterContextInvokeData
5858
{
59-
CHIP_ERROR (*mFunc)(void *);
60-
void * mFuncUserData;
61-
CHIP_ERROR mFuncResult;
59+
LambdaBridge bridge;
6260
// Sync primitives to wait for the function to be executed
6361
std::mutex mDoneMutex;
6462
std::condition_variable mDoneCond;
@@ -144,18 +142,17 @@ void PlatformManagerImpl::_Shutdown()
144142
mGLibMainLoop = nullptr;
145143
}
146144

147-
CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(void *), void * userData)
145+
void PlatformManagerImpl::_GLibMatterContextInvokeSync(LambdaBridge && bridge)
148146
{
149-
GLibMatterContextInvokeData invokeData{ func, userData };
147+
GLibMatterContextInvokeData invokeData{ std::move(bridge) };
150148

151149
g_main_context_invoke_full(
152150
g_main_loop_get_context(mGLibMainLoop), G_PRIORITY_HIGH_IDLE,
153151
[](void * userData_) {
154152
auto * data = reinterpret_cast<GLibMatterContextInvokeData *>(userData_);
155153
VerifyOrExit(g_main_context_get_thread_default() != nullptr,
156-
ChipLogError(DeviceLayer, "GLib thread default main context is not set");
157-
data->mFuncResult = CHIP_ERROR_INCORRECT_STATE);
158-
data->mFuncResult = data->mFunc(data->mFuncUserData);
154+
ChipLogError(DeviceLayer, "GLib thread default main context is not set"));
155+
data->bridge();
159156
exit:
160157
data->mDoneMutex.lock();
161158
data->mDone = true;
@@ -167,8 +164,6 @@ CHIP_ERROR PlatformManagerImpl::_GLibMatterContextInvokeSync(CHIP_ERROR (*func)(
167164

168165
std::unique_lock<std::mutex> lock(invokeData.mDoneMutex);
169166
invokeData.mDoneCond.wait(lock, [&invokeData]() { return invokeData.mDone; });
170-
171-
return invokeData.mFuncResult;
172167
}
173168

174169
} // namespace DeviceLayer

‎src/platform/Tizen/PlatformManagerImpl.h

+21-4
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,21 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
6464
template <typename T>
6565
CHIP_ERROR GLibMatterContextInvokeSync(CHIP_ERROR (*func)(T *), T * userData)
6666
{
67-
return _GLibMatterContextInvokeSync((CHIP_ERROR(*)(void *)) func, (void *) userData);
67+
struct
68+
{
69+
CHIP_ERROR returnValue = CHIP_NO_ERROR;
70+
CHIP_ERROR (*functionToCall)(T *);
71+
T * userData;
72+
} context;
73+
74+
context.functionToCall = func;
75+
context.userData = userData;
76+
77+
LambdaBridge bridge;
78+
bridge.Initialize([&context]() { context.returnValue = context.functionToCall(context.userData); });
79+
80+
_GLibMatterContextInvokeSync(std::move(bridge));
81+
return context.returnValue;
6882
}
6983
System::Clock::Timestamp GetStartTime() { return mStartTime; }
7084

@@ -87,10 +101,13 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
87101
/**
88102
* @brief Invoke a function on the Matter GLib context.
89103
*
90-
* @note This function does not provide type safety for the user data. Please,
91-
* use the GLibMatterContextInvokeSync() template function instead.
104+
* @param[in] bridge a LambdaBridge object that holds the lambda to be invoked within the GLib context.
105+
*
106+
* @note This function moves the LambdaBridge into the GLib context for invocation.
107+
* The LambdaBridge is created and initialised in GLibMatterContextInvokeSync().
108+
* use the GLibMatterContextInvokeSync() template function instead of this one.
92109
*/
93-
CHIP_ERROR _GLibMatterContextInvokeSync(CHIP_ERROR (*func)(void *), void * userData);
110+
void _GLibMatterContextInvokeSync(LambdaBridge && bridge);
94111

95112
GMainLoop * mGLibMainLoop;
96113
GThread * mGLibMainLoopThread;

0 commit comments

Comments
 (0)