Assert that the Matter stack lock is held while reading/writing attributes

config/qpg/chip-gn/args.gni

@@ -32,6 +32,10 @@ qpg_cc = "arm-none-eabi-gcc"
 qpg_cxx = "arm-none-eabi-g++"
 chip_mdns = "platform"
 
+# Disable lock tracking, since our FreeRTOS configuration does not set
+# INCLUDE_xSemaphoreGetMutexHolder
+chip_stack_lock_tracking = "none"
+
 chip_project_config_include_dirs =
     [ "//examples/platform/qpg/project_include/" ]
 chip_project_config_include = "<CHIPProjectConfig.h>"

examples/all-clusters-app/p6/args.gni

@@ -18,3 +18,9 @@ import("${chip_root}/src/platform/P6/args.gni")
 
 p6_target_project =
     get_label_info(":all_clusters_app_sdk_sources", "label_no_toolchain")
+
+declare_args() {
+  # Disable lock tracking, since our FreeRTOS configuration does not set
+  # INCLUDE_xSemaphoreGetMutexHolder
+  chip_stack_lock_tracking = "none"
+}

examples/lighting-app/p6/args.gni

@@ -18,3 +18,9 @@ import("${chip_root}/src/platform/P6/args.gni")
 
 p6_target_project =
     get_label_info(":lighting_app_sdk_sources", "label_no_toolchain")
+
+declare_args() {
+  # Disable lock tracking, since our FreeRTOS configuration does not set
+  # INCLUDE_xSemaphoreGetMutexHolder
+  chip_stack_lock_tracking = "none"
+}

examples/lighting-app/qpg/args.gni

@@ -20,6 +20,10 @@ qpg_sdk_target = get_label_info(":sdk", "label_no_toolchain")
 
 declare_args() {
   chip_enable_ota_requestor = true
+
+  # Disable lock tracking, since our FreeRTOS configuration does not set
+  # INCLUDE_xSemaphoreGetMutexHolder
+  chip_stack_lock_tracking = "none"
 }
 
 pw_log_BACKEND = "${chip_root}/src/lib/support/pw_log_chip"

examples/lock-app/p6/args.gni

@@ -18,3 +18,9 @@ import("${chip_root}/src/platform/P6/args.gni")
 
 p6_target_project =
     get_label_info(":lock_app_sdk_sources", "label_no_toolchain")
+
+declare_args() {
+  # Disable lock tracking, since our FreeRTOS configuration does not set
+  # INCLUDE_xSemaphoreGetMutexHolder
+  chip_stack_lock_tracking = "none"
+}

examples/lock-app/qpg/args.gni

@@ -20,6 +20,10 @@ qpg_sdk_target = get_label_info(":sdk", "label_no_toolchain")
 
 declare_args() {
   chip_enable_ota_requestor = true
+
+  # Disable lock tracking, since our FreeRTOS configuration does not set
+  # INCLUDE_xSemaphoreGetMutexHolder
+  chip_stack_lock_tracking = "none"
 }
 
 pw_log_BACKEND = "${chip_root}/src/lib/support/pw_log_chip"

examples/persistent-storage/qpg/args.gni

@@ -17,3 +17,9 @@ import("//build_overrides/chip.gni")
 import("${chip_root}/examples/platform/qpg/args.gni")
 qpg_sdk_target = get_label_info(":sdk", "label_no_toolchain")
 chip_enable_openthread = false
+
+declare_args() {
+  # Disable lock tracking, since our FreeRTOS configuration does not set
+  # INCLUDE_xSemaphoreGetMutexHolder
+  chip_stack_lock_tracking = "none"
+}

src/app/util/attribute-storage.cpp

@@ -47,6 +47,7 @@
 #include <app/util/attribute-storage.h>
 #include <lib/support/CodeUtils.h>
 #include <lib/support/logging/CHIPLogging.h>
+#include <platform/LockTracker.h>
 
 #include <app-common/zap-generated/attribute-type.h>
 #include <app-common/zap-generated/callback.h>
@@ -549,6 +550,8 @@ bool emAfMatchAttribute(const EmberAfCluster * cluster, const EmberAfAttributeMe
 EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord, const EmberAfAttributeMetadata ** metadata,
                                        uint8_t * buffer, uint16_t readLength, bool write)
 {
+    assertChipStackLockedByCurrentThread();
+
     uint16_t attributeOffsetIndex = 0;
 
     for (uint8_t ep = 0; ep < emberAfEndpointCount(); ep++)

src/app/util/ember-compatibility-functions.cpp

@@ -42,6 +42,7 @@
 #include <lib/support/CodeUtils.h>
 #include <lib/support/SafeInt.h>
 #include <lib/support/TypeTraits.h>
+#include <platform/LockTracker.h>
 #include <protocols/interaction_model/Constants.h>
 
 #include <app-common/zap-generated/att-storage.h>
@@ -1029,6 +1030,10 @@ void MatterReportingAttributeChangeCallback(EndpointId endpoint, ClusterId clust
 
 void MatterReportingAttributeChangeCallback(EndpointId endpoint, ClusterId clusterId, AttributeId attributeId)
 {
+    // Attribute writes have asserted this already, but this assert should catch
+    // applications notifying about changes from their end.
+    assertChipStackLockedByCurrentThread();
+
     ClusterInfo info;
     info.mClusterId   = clusterId;
     info.mAttributeId = attributeId;

src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.cpp

@@ -104,6 +104,27 @@ void GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_UnlockChipStack(void)
     xSemaphoreGive(mChipStackLock);
 }
 
+#if CHIP_STACK_LOCK_TRACKING_ENABLED
+template <class ImplClass>
+bool GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_IsChipStackLockedByCurrentThread() const
+{
+    // We can't check for INCLUDE_xTaskGetCurrentTaskHandle because it's often
+    // _not_ set, but xTaskGetCurrentTaskHandle works anyway because
+    // configUSE_MUTEXES is set.  So in practice, xTaskGetCurrentTaskHandle can
+    // be assumed to be available here.
+#if INCLUDE_xSemaphoreGetMutexHolder != 1
+#error Must either set INCLUDE_xSemaphoreGetMutexHolder = 1 in FreeRTOSConfig.h or set chip_stack_lock_tracking = "none" in Matter gn configuration.
+#endif
+    // If we have not started our event loop yet, return true because in that
+    // case we can't be racing against the (not yet started) event loop.
+    //
+    // Similarly, if mChipStackLock has not been created yet, might as well
+    // return true.
+    return (mEventLoopTask == nullptr) || (mChipStackLock == nullptr) ||
+        (xSemaphoreGetMutexHolder(mChipStackLock) == xTaskGetCurrentTaskHandle());
+}
+#endif // CHIP_STACK_LOCK_TRACKING_ENABLED
+
 template <class ImplClass>
 CHIP_ERROR GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_PostEvent(const ChipDeviceEvent * event)
 {

src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h

@@ -78,6 +78,10 @@ class GenericPlatformManagerImpl_FreeRTOS : public GenericPlatformManagerImpl<Im
     CHIP_ERROR _StartChipTimer(System::Clock::Timeout duration);
     CHIP_ERROR _Shutdown(void);
 
+#if CHIP_STACK_LOCK_TRACKING_ENABLED
+    bool _IsChipStackLockedByCurrentThread() const;
+#endif // CHIP_STACK_LOCK_TRACKING_ENABLED
+
     // ===== Methods available to the implementation subclass.
 
     void PostEventFromISR(const ChipDeviceEvent * event, BaseType_t & yieldRequired);

src/platform/BUILD.gn

@@ -69,9 +69,9 @@ if (chip_device_platform != "none") {
 
   if (chip_stack_lock_tracking == "auto") {
     if (chip_device_platform == "linux" || chip_device_platform == "tizen" ||
-        chip_device_platform == "android") {
+        chip_device_platform == "android" || current_os == "freertos") {
       # TODO: should be fatal for development. Change once bugs are fixed
-      chip_stack_lock_tracking = "log"
+      chip_stack_lock_tracking = "fatal"
     } else {
       # TODO: may want to enable at least logging for embedded to find bugs
       # this needs tuning depending on how many resources various platforms have