Skip to content

Commit

Permalink
Assert the Matter lock is held in non-threadsafe SystemLayer methods. (
Browse files Browse the repository at this point in the history
…#26180)

ScheduleWork uses timers, and timers access a timer list member that is not
protected against data races in any way.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Oct 17, 2023
1 parent 47f9470 commit 1240900
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 4 deletions.
12 changes: 8 additions & 4 deletions src/system/SystemLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ class DLL_EXPORT Layer

/**
* @brief
* This method starts a one-shot timer.
* This method starts a one-shot timer. This method must be called while in the Matter context (from
* the Matter event loop, or while holding the Matter stack lock).
*
* @note
* Only a single timer is allowed to be started with the same @a aComplete and @a aAppState
Expand All @@ -114,8 +115,9 @@ class DLL_EXPORT Layer
virtual CHIP_ERROR StartTimer(Clock::Timeout aDelay, TimerCompleteCallback aComplete, void * aAppState) = 0;

/**
* @brief
* This method cancels a one-shot timer, started earlier through @p StartTimer().
* @brief This method cancels a one-shot timer, started earlier through @p StartTimer(). This method must
* be called while in the Matter context (from the Matter event loop, or while holding the Matter
* stack lock).
*
* @note
* The cancellation could fail silently if the timer specified by the combination of the callback
Expand All @@ -129,7 +131,9 @@ class DLL_EXPORT Layer

/**
* @brief
* Schedules a function with a signature identical to `OnCompleteFunct` to be run as soon as possible in the CHIP context.
* Schedules a function with a signature identical to `OnCompleteFunct` to be run as soon as possible in the Matter context.
* This must only be called when already in the Matter context (from the Matter event loop, or while holding the Matter
* stack lock).
*
* @param[in] aComplete A pointer to a callback function to be called when this timer fires.
* @param[in] aAppState A pointer to an application state object to be passed to the callback function as argument.
Expand Down
7 changes: 7 additions & 0 deletions src/system/SystemLayerImplFreeRTOS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
*/

#include <lib/support/CodeUtils.h>
#include <platform/LockTracker.h>
#include <system/PlatformEventSupport.h>
#include <system/SystemFaultInjection.h>
#include <system/SystemLayer.h>
Expand Down Expand Up @@ -51,6 +52,8 @@ void LayerImplFreeRTOS::Shutdown()

CHIP_ERROR LayerImplFreeRTOS::StartTimer(Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState)
{
assertChipStackLockedByCurrentThread();

VerifyOrReturnError(mLayerState.IsInitialized(), CHIP_ERROR_INCORRECT_STATE);

CHIP_SYSTEM_FAULT_INJECT(FaultInjection::kFault_TimeoutImmediate, delay = Clock::kZero);
Expand All @@ -75,6 +78,8 @@ CHIP_ERROR LayerImplFreeRTOS::StartTimer(Clock::Timeout delay, TimerCompleteCall

void LayerImplFreeRTOS::CancelTimer(TimerCompleteCallback onComplete, void * appState)
{
assertChipStackLockedByCurrentThread();

VerifyOrReturn(mLayerState.IsInitialized());

TimerList::Node * timer = mTimerList.Remove(onComplete, appState);
Expand All @@ -86,6 +91,8 @@ void LayerImplFreeRTOS::CancelTimer(TimerCompleteCallback onComplete, void * app

CHIP_ERROR LayerImplFreeRTOS::ScheduleWork(TimerCompleteCallback onComplete, void * appState)
{
assertChipStackLockedByCurrentThread();

VerifyOrReturnError(mLayerState.IsInitialized(), CHIP_ERROR_INCORRECT_STATE);

// Ideally we would not use a timer here at all, but if we try to just
Expand Down
6 changes: 6 additions & 0 deletions src/system/SystemLayerImplSelect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ void LayerImplSelect::Signal()

CHIP_ERROR LayerImplSelect::StartTimer(Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState)
{
assertChipStackLockedByCurrentThread();

VerifyOrReturnError(mLayerState.IsInitialized(), CHIP_ERROR_INCORRECT_STATE);

CHIP_SYSTEM_FAULT_INJECT(FaultInjection::kFault_TimeoutImmediate, delay = System::Clock::kZero);
Expand Down Expand Up @@ -167,6 +169,8 @@ CHIP_ERROR LayerImplSelect::StartTimer(Clock::Timeout delay, TimerCompleteCallba

void LayerImplSelect::CancelTimer(TimerCompleteCallback onComplete, void * appState)
{
assertChipStackLockedByCurrentThread();

VerifyOrReturn(mLayerState.IsInitialized());

TimerList::Node * timer = mTimerList.Remove(onComplete, appState);
Expand Down Expand Up @@ -194,6 +198,8 @@ void LayerImplSelect::CancelTimer(TimerCompleteCallback onComplete, void * appSt

CHIP_ERROR LayerImplSelect::ScheduleWork(TimerCompleteCallback onComplete, void * appState)
{
assertChipStackLockedByCurrentThread();

VerifyOrReturnError(mLayerState.IsInitialized(), CHIP_ERROR_INCORRECT_STATE);

#if CHIP_SYSTEM_CONFIG_USE_DISPATCH
Expand Down

0 comments on commit 1240900

Please sign in to comment.