Fix RPC deadlock on Linux

examples/common/pigweed/rpc_services/Attributes.h

@@ -28,30 +28,19 @@
 #include <lib/core/TLVTags.h>
 #include <lib/core/TLVTypes.h>
 #include <platform/PlatformManager.h>
-#include <future>
 
 namespace chip {
 namespace rpc {
 
-struct WriteArgs {
-    std::promise<EmberAfStatus> *promise;
-    const chip_rpc_AttributeWrite *request;
-    const void *data;
-};
-
-struct ReadArgs {
-    std::promise<CHIP_ERROR> *promise;
-    const app::ConcreteAttributePath *path;
-    app::AttributeReportIBs::Builder *attributeReports;
-};
-
 // Implementation class for chip.rpc.Attributes.
 class Attributes : public pw_rpc::nanopb::Attributes::Service<Attributes>
 {
 public:
     ::pw::Status Write(const chip_rpc_AttributeWrite & request, pw_protobuf_Empty & response)
     {
         const void * data;
+        DeviceLayer::StackLock lock;
+
         switch (request.data.which_data)
         {
         case chip_rpc_AttributeData_data_bool_tag:
@@ -81,28 +70,9 @@ class Attributes : public pw_rpc::nanopb::Attributes::Service<Attributes>
         default:
             return pw::Status::InvalidArgument();
         }
-
-        std::promise<EmberAfStatus> writeStatusPromise;
-        WriteArgs writeArgs = {
-            .promise = &writeStatusPromise,
-            .request = &request,
-            .data = data,
-        };
-        chip::DeviceLayer::PlatformMgr().ScheduleWork([](intptr_t ctx) {
-            DeviceLayer::StackLock lock;
-
-            WriteArgs *args = reinterpret_cast<WriteArgs*>(ctx);
-            args->promise->set_value(
-                 emberAfWriteAttribute(
-                     args->request->metadata.endpoint,
-                     args->request->metadata.cluster,
-                     args->request->metadata.attribute_id,
-                     const_cast<uint8_t *>(static_cast<const uint8_t *>(args->data)),
-                     args->request->metadata.type));
-        }, reinterpret_cast<intptr_t>(&writeArgs));
-
-        RETURN_STATUS_IF_NOT_OK(writeStatusPromise.get_future().get());
-
+        RETURN_STATUS_IF_NOT_OK(
+            emberAfWriteAttribute(request.metadata.endpoint, request.metadata.cluster, request.metadata.attribute_id,
+                                  const_cast<uint8_t *>(static_cast<const uint8_t *>(data)), request.metadata.type));
         return pw::OkStatus();
     }
 
@@ -220,6 +190,7 @@ class Attributes : public pw_rpc::nanopb::Attributes::Service<Attributes>
 
     ::pw::Status ReadAttributeIntoTlvBuffer(const app::ConcreteAttributePath & path, MutableByteSpan & tlvBuffer)
     {
+        Access::SubjectDescriptor subjectDescriptor{ .authMode = chip::Access::AuthMode::kPase };
         app::AttributeReportIBs::Builder attributeReports;
         TLV::TLVWriter writer;
         TLV::TLVType outer;
@@ -228,21 +199,7 @@ class Attributes : public pw_rpc::nanopb::Attributes::Service<Attributes>
         writer.Init(tlvBuffer);
         PW_TRY(ChipErrorToPwStatus(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outer)));
         PW_TRY(ChipErrorToPwStatus(attributeReports.Init(&writer, kReportContextTag)));
-
-        std::promise<CHIP_ERROR> readStatusPromise;
-        ReadArgs readArgs = {
-            .promise = &readStatusPromise,
-            .path = &path,
-            .attributeReports = &attributeReports,
-        };
-        chip::DeviceLayer::PlatformMgr().ScheduleWork([](intptr_t ctx) {
-            Access::SubjectDescriptor subjectDescriptor{ .authMode = chip::Access::AuthMode::kPase };
-            ReadArgs *args = reinterpret_cast<ReadArgs*>(ctx);
-            args->promise->set_value(app::ReadSingleClusterData(subjectDescriptor, false, *(args->path), *(args->attributeReports), nullptr));
-        }, reinterpret_cast<intptr_t>(&readArgs));
-
-        PW_TRY(ChipErrorToPwStatus(readStatusPromise.get_future().get()));
-
+        PW_TRY(ChipErrorToPwStatus(app::ReadSingleClusterData(subjectDescriptor, false, path, attributeReports, nullptr)));
         attributeReports.EndOfContainer();
         PW_TRY(ChipErrorToPwStatus(writer.EndContainer(outer)));
         PW_TRY(ChipErrorToPwStatus(writer.Finalize()));

src/platform/Darwin/PlatformManagerImpl.cpp

@@ -159,7 +159,7 @@ bool PlatformManagerImpl::_IsChipStackLockedByCurrentThread() const
 {
     // If we have no work queue, or it's suspended, then we assume our caller
     // knows what they are doing in terms of their own concurrency.
-    return !mWorkQueue || mIsWorkQueueSuspended || IsWorkQueueCurrentQueue();
+    return !mWorkQueue || mIsWorkQueueSuspended || IsWorkQueueCurrentQueue() || (mChipStackIsLocked && (pthread_equal(pthread_self(), mChipStackLockOwnerThread)));
 };
 #endif
 

src/platform/Darwin/PlatformManagerImpl.h

@@ -27,6 +27,8 @@
 #include <platform/internal/GenericPlatformManagerImpl.h>
 
 #include <dispatch/dispatch.h>
+#include <mutex>
+#include <pthread.h>
 
 static constexpr const char * const CHIP_CONTROLLER_QUEUE = "org.csa-iot.matter.framework.controller.workqueue";
 
@@ -66,12 +68,37 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
     CHIP_ERROR _StopEventLoopTask();
 
     void _RunEventLoop();
-    void _LockChipStack(){};
-    bool _TryLockChipStack() { return false; };
-    void _UnlockChipStack(){};
+    void _LockChipStack()
+    {
+        mLock.lock();
+#if CHIP_STACK_LOCK_TRACKING_ENABLED
+        mChipStackIsLocked = true;
+        mChipStackLockOwnerThread = pthread_self();
+#endif
+    };
+    bool _TryLockChipStack()
+    {
+        bool isLockSuccessful = mLock.try_lock();
+#if CHIP_STACK_LOCK_TRACKING_ENABLED
+        if (isLockSuccessful) {
+           mChipStackIsLocked = true;
+           mChipStackLockOwnerThread = pthread_self();
+        }
+        return isLockSuccessful;
+#endif
+    };
+    void _UnlockChipStack()
+    {
+#if CHIP_STACK_LOCK_TRACKING_ENABLED
+        mChipStackIsLocked = false;
+#endif
+        mLock.unlock();
+    };
     CHIP_ERROR _PostEvent(const ChipDeviceEvent * event);
 
 #if CHIP_STACK_LOCK_TRACKING_ENABLED
+    bool mChipStackIsLocked = false;
+    pthread_t mChipStackLockOwnerThread;
     bool _IsChipStackLockedByCurrentThread() const;
 #endif
 
@@ -93,6 +120,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
     // atomic ops, if we're worried about calls to StopEventLoopTask() from
     // multiple threads racing somehow...
     bool mIsWorkQueueSuspensionPending = false;
+    std::mutex mLock;
 
     inline ImplClass * Impl() { return static_cast<PlatformManagerImpl *>(this); }
 };