From 0b7ee89525c50dd493fbe6a07a0effc50f804244 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 30 Jun 2022 10:17:08 -0700 Subject: [PATCH 1/3] Assert that controllers are inited and shut down with the Matter lock held. We were not asserting this, and some consumers were not doing this, leading to data races. --- src/controller/CHIPDeviceController.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 2991a32dbbb3cb..8422f5508d4bf3 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -64,6 +64,7 @@ #include #include #include +#include #include #include #include @@ -105,6 +106,8 @@ DeviceController::DeviceController() CHIP_ERROR DeviceController::Init(ControllerInitParams params) { + assertChipStackLockedByCurrentThread(); + VerifyOrReturnError(mState == State::NotInitialized, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(params.systemState != nullptr, CHIP_ERROR_INVALID_ARGUMENT); @@ -282,6 +285,8 @@ CHIP_ERROR DeviceController::InitControllerNOCChain(const ControllerInitParams & void DeviceController::Shutdown() { + assertChipStackLockedByCurrentThread(); + VerifyOrReturn(mState != State::NotInitialized); ChipLogDetail(Controller, "Shutting down the controller"); From 9fbaf82f72e22af962270a86d203d025a4a2871e Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 30 Jun 2022 11:09:27 -0700 Subject: [PATCH 2/3] Fix platform manager impls to correctly flag tasks as shut down. --- .../internal/GenericPlatformManagerImpl_FreeRTOS.ipp | 8 ++++++++ .../platform/internal/GenericPlatformManagerImpl_POSIX.h | 1 - .../internal/GenericPlatformManagerImpl_POSIX.ipp | 3 +-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.ipp b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.ipp index 6bbff042779470..6e5a17b5086c3b 100644 --- a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.ipp +++ b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.ipp @@ -215,6 +215,14 @@ void GenericPlatformManagerImpl_FreeRTOS::_RunEventLoop(void) eventReceived = xQueueReceive(mChipEventQueue, &event, 0); } } + +#if defined(INCLUDE_vTaskDelete) && INCLUDE_vTaskDelete + if (mEventLoopTask) + { + vTaskDelete(mEventLoopTask); + mEventLoopTask = nullptr; + } +#endif // INCLUDE_vTaskDelete } template diff --git a/src/include/platform/internal/GenericPlatformManagerImpl_POSIX.h b/src/include/platform/internal/GenericPlatformManagerImpl_POSIX.h index bf04f5498d2d88..7fd248fdf699d9 100644 --- a/src/include/platform/internal/GenericPlatformManagerImpl_POSIX.h +++ b/src/include/platform/internal/GenericPlatformManagerImpl_POSIX.h @@ -77,7 +77,6 @@ class GenericPlatformManagerImpl_POSIX : public GenericPlatformManagerImpl::_UnlockChipStack() template bool GenericPlatformManagerImpl_POSIX::_IsChipStackLockedByCurrentThread() const { - return !mMainLoopStarted || (mChipStackIsLocked && (pthread_equal(pthread_self(), mChipStackLockOwnerThread))); + return !mHasValidChipTask || (mChipStackIsLocked && (pthread_equal(pthread_self(), mChipStackLockOwnerThread))); } #endif @@ -201,7 +201,6 @@ template void * GenericPlatformManagerImpl_POSIX::EventLoopTaskMain(void * arg) { ChipLogDetail(DeviceLayer, "CHIP task running"); - static_cast *>(arg)->Impl()->mMainLoopStarted = true; static_cast *>(arg)->Impl()->RunEventLoop(); return nullptr; } From 6fa69e737e6c2b411aa94d0ac1898e5a31e5d59a Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 30 Jun 2022 11:35:33 -0700 Subject: [PATCH 3/3] Undo the FreeRTOS changes. --- .../internal/GenericPlatformManagerImpl_FreeRTOS.ipp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.ipp b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.ipp index 6e5a17b5086c3b..6bbff042779470 100644 --- a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.ipp +++ b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.ipp @@ -215,14 +215,6 @@ void GenericPlatformManagerImpl_FreeRTOS::_RunEventLoop(void) eventReceived = xQueueReceive(mChipEventQueue, &event, 0); } } - -#if defined(INCLUDE_vTaskDelete) && INCLUDE_vTaskDelete - if (mEventLoopTask) - { - vTaskDelete(mEventLoopTask); - mEventLoopTask = nullptr; - } -#endif // INCLUDE_vTaskDelete } template