Add IM Event Lock support (#7332)

project-chip · Aug 20, 2021 · 3111261 · 3111261
1 parent c928fde
commit 3111261
Show file tree

Hide file tree

Showing 16 changed files with 57 additions and 215 deletions.
diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
@@ -20,20 +20,6 @@ declare_args() {
   # Enable strict schema checks.
   chip_enable_schema_check =
       is_debug && (current_os == "linux" || current_os == "mac")
-
-  # Mutex implementation: posix, freertos, zephyr.
-  chip_im_config_critical_section = ""
-}
-
-# TODO: Add locker support for different platform
-if (chip_im_config_critical_section == "") {
-  if (current_os == "freertos") {
-    chip_im_config_critical_section = "rtos"
-  } else if (current_os == "zephyr") {
-    chip_im_config_critical_section = "zephyr"
-  } else {
-    chip_im_config_critical_section = "posix"
-  }
 }
 
 config("app_config") {
@@ -110,16 +96,6 @@ static_library("app") {
     "reporting/Engine.h",
   ]
 
-  if (chip_im_config_critical_section == "posix") {
-    sources += [ "CriticalSectionImplPosix.cpp" ]
-  } else if (chip_im_config_critical_section == "rtos") {
-    sources += [ "CriticalSectionImplRtos.cpp" ]
-  } else if (chip_im_config_critical_section == "zephyr") {
-    sources += [ "CriticalSectionImplZephyr.cpp" ]
-  } else {
-    assert(false, "Unknown IM critical section implementation.")
-  }
-
   if (chip_ip_commissioning) {
     defines = [
       "CONFIG_USE_CLUSTERS_FOR_IP_COMMISSIONING=1",

diff --git a/src/app/CriticalSection.h b/src/app/CriticalSection.h
diff --git a/src/app/CriticalSectionImplPosix.cpp b/src/app/CriticalSectionImplPosix.cpp
diff --git a/src/app/CriticalSectionImplRtos.cpp b/src/app/CriticalSectionImplRtos.cpp
diff --git a/src/app/CriticalSectionImplZephyr.cpp b/src/app/CriticalSectionImplZephyr.cpp
diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp
@@ -16,7 +16,6 @@
  *    limitations under the License.
  */
 
-#include <app/CriticalSection.h>
 #include <app/EventManagement.h>
 #include <app/InteractionModelEngine.h>
 #include <core/CHIPEventLoggingConfig.h>
@@ -98,25 +97,28 @@ struct EventEnvelopeContext
     EventId mEventId           = 0;
 };
 
-EventManagement::EventManagement(Messaging::ExchangeManager * apExchangeMgr, int aNumBuffers,
-                                 CircularEventBuffer * apCircularEventBuffer,
-                                 const LogStorageResources * const apLogStorageResources)
-{
-    Init(apExchangeMgr, aNumBuffers, apCircularEventBuffer, apLogStorageResources);
-}
-
-void EventManagement::Init(Messaging::ExchangeManager * apExchangeManager, int aNumBuffers,
+void EventManagement::Init(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers,
                            CircularEventBuffer * apCircularEventBuffer, const LogStorageResources * const apLogStorageResources)
 {
+    CHIP_ERROR err                = CHIP_NO_ERROR;
     CircularEventBuffer * current = nullptr;
     CircularEventBuffer * prev    = nullptr;
     CircularEventBuffer * next    = nullptr;
 
-    VerifyOrDie(aNumBuffers > 0);
+    if (aNumBuffers == 0)
+    {
+        ChipLogError(EventLogging, "Invalid aNumBuffers");
+        return;
+    }
 
+    if (mState != EventManagementStates::Shutdown)
+    {
+        ChipLogError(EventLogging, "Invalid EventManagement State");
+        return;
+    }
     mpExchangeMgr = apExchangeManager;
 
-    for (int bufferIndex = 0; bufferIndex < aNumBuffers; bufferIndex++)
+    for (uint32_t bufferIndex = 0; bufferIndex < aNumBuffers; bufferIndex++)
     {
         next = (bufferIndex < aNumBuffers - 1) ? &apCircularEventBuffer[bufferIndex + 1] : nullptr;
 
@@ -134,6 +136,12 @@ void EventManagement::Init(Messaging::ExchangeManager * apExchangeManager, int a
     mpEventBuffer = apCircularEventBuffer;
     mState        = EventManagementStates::Idle;
     mBytesWritten = 0;
+
+    err = chip::System::Mutex::Init(mAccessLock);
+    if (err != CHIP_NO_ERROR)
+    {
+        ChipLogError(EventLogging, "mutex init fails with error %s", ErrorStr(err));
+    }
 }
 
 CHIP_ERROR EventManagement::CopyToNextBuffer(CircularEventBuffer * apEventBuffer)
@@ -361,25 +369,23 @@ CHIP_ERROR EventManagement::ConstructEvent(EventLoadOutContext * apContext, Even
     return err;
 }
 
-void EventManagement::CreateEventManagement(Messaging::ExchangeManager * apExchangeManager, int aNumBuffers,
+void EventManagement::CreateEventManagement(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers,
                                             CircularEventBuffer * apCircularEventBuffer,
                                             const LogStorageResources * const apLogStorageResources)
 {
 
     sInstance.Init(apExchangeManager, aNumBuffers, apCircularEventBuffer, apLogStorageResources);
-    static_assert(std::is_trivially_destructible<EventManagement>::value, "EventManagement must be trivially destructible");
 }
 
 /**
  * @brief Perform any actions we need to on shutdown.
  */
 void EventManagement::DestroyEventManagement()
 {
-    CriticalSectionEnter();
+    ScopedLock lock(sInstance);
     sInstance.mState        = EventManagementStates::Shutdown;
     sInstance.mpEventBuffer = nullptr;
     sInstance.mpExchangeMgr = nullptr;
-    CriticalSectionExit();
 }
 
 EventNumber CircularEventBuffer::VendEventNumber()
@@ -462,15 +468,15 @@ CHIP_ERROR EventManagement::CopyAndAdjustDeltaTime(const TLVReader & aReader, si
 CHIP_ERROR EventManagement::LogEvent(EventLoggingDelegate * apDelegate, EventOptions & aEventOptions, EventNumber & aEventNumber)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
-    CriticalSectionEnter();
-
-    VerifyOrExit(mState != EventManagementStates::Shutdown, err = CHIP_ERROR_INCORRECT_STATE);
-    VerifyOrExit(aEventOptions.mpEventSchema != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
+    {
+        ScopedLock lock(sInstance);
 
-    err = LogEventPrivate(apDelegate, aEventOptions, aEventNumber);
+        VerifyOrExit(mState != EventManagementStates::Shutdown, err = CHIP_ERROR_INCORRECT_STATE);
+        VerifyOrExit(aEventOptions.mpEventSchema != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
 
+        err = LogEventPrivate(apDelegate, aEventOptions, aEventNumber);
+    }
 exit:
-    CriticalSectionExit();
     ChipLogFunctError(err);
     return err;
 }
@@ -692,7 +698,7 @@ CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, ClusterInfo *
     EventLoadOutContext context(aWriter, aPriority, aEventNumber);
 
     CircularEventBuffer * buf = mpEventBuffer;
-    CriticalSectionEnter();
+    ScopedLock lock(sInstance);
 
     while (!buf->IsFinalDestinationForPriority(aPriority))
     {
@@ -713,7 +719,6 @@ CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, ClusterInfo *
 
 exit:
     aEventNumber = context.mCurrentEventNumber;
-    CriticalSectionExit();
 
     return err;
 }
@@ -832,7 +837,7 @@ void EventManagement::SetScheduledEventEndpoint(EventNumber * apEventEndpoints)
 {
     CircularEventBuffer * eventBuffer = mpEventBuffer;
 
-    CriticalSectionEnter();
+    ScopedLock lock(sInstance);
 
     while (eventBuffer != nullptr)
     {
@@ -842,8 +847,6 @@ void EventManagement::SetScheduledEventEndpoint(EventNumber * apEventEndpoints)
         }
         eventBuffer = eventBuffer->GetNextCircularEventBuffer();
     }
-
-    CriticalSectionExit();
 }
 
 void CircularEventBuffer::Init(uint8_t * apBuffer, uint32_t aBufferLength, CircularEventBuffer * apPrev,

diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h
@@ -34,6 +34,7 @@
 #include <core/CHIPCircularTLVBuffer.h>
 #include <messaging/ExchangeMgr.h>
 #include <support/PersistedCounter.h>
+#include <system/SystemMutex.h>
 
 #define CHIP_CONFIG_EVENT_GLOBAL_PRIORITY PriorityLevel::Debug
 
@@ -241,8 +242,6 @@ class EventManagement
 public:
     /**
      * @brief
-     *   EventManagement constructor
-     *
      * Initialize the EventManagement with an array of LogStorageResources.  The
      * array must provide a resource for each valid priority level, the elements
      * of the array must be in increasing numerical value of priority (and in
@@ -259,16 +258,8 @@ class EventManagement
      * @param[in] apLogStorageResources  An array of LogStorageResources for each priority level.
      *
      */
-    EventManagement(Messaging::ExchangeManager * apExchangeManager, int aNumBuffers, CircularEventBuffer * apCircularEventBuffer,
-                    const LogStorageResources * const apLogStorageResources);
-
-    void Init(Messaging::ExchangeManager * apExchangeManager, int aNumBuffers, CircularEventBuffer * apCircularEventBuffer,
+    void Init(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer,
               const LogStorageResources * const apLogStorageResources);
-    /**
-     * @brief
-     *   EventManagement default constructor. Provided primarily to make the compiler happy.
-     */
-    EventManagement(){};
 
     static EventManagement & GetInstance();
 
@@ -292,12 +283,21 @@ class EventManagement
      *
      * @note This function must be called prior to the logging being used.
      */
-    static void CreateEventManagement(Messaging::ExchangeManager * apExchangeManager, int aNumBuffers,
+    static void CreateEventManagement(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers,
                                       CircularEventBuffer * apCircularEventBuffer,
                                       const LogStorageResources * const apLogStorageResources);
 
     static void DestroyEventManagement();
 
+    class ScopedLock
+    {
+    public:
+        ScopedLock(EventManagement & aEventManagement) : mEventManagement(aEventManagement) { mEventManagement.mAccessLock.Lock(); }
+        ~ScopedLock() { mEventManagement.mAccessLock.Unlock(); }
+
+    private:
+        EventManagement & mEventManagement;
+    };
     /**
      * @brief
      *   Log an event via a EventLoggingDelegate, with options.
@@ -566,6 +566,7 @@ class EventManagement
     Messaging::ExchangeManager * mpExchangeMgr = nullptr;
     EventManagementStates mState               = EventManagementStates::Shutdown;
     uint32_t mBytesWritten                     = 0;
+    System::Mutex mAccessLock;
 };
 } // namespace app
 } // namespace chip
diff --git a/src/inet/InetLayer.h b/src/inet/InetLayer.h
@@ -92,7 +92,11 @@
 #endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING
 
 #if CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING
+#if defined(ESP_PLATFORM)
+#include "freertos/FreeRTOS.h"
+#else
 #include <FreeRTOS.h>
+#endif
 #include <semphr.h>
 #endif // CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING