diff options
author | Yu Shan <shanyu@google.com> | 2022-12-08 11:39:26 -0800 |
---|---|---|
committer | Yu Shan <shanyu@google.com> | 2022-12-09 21:41:59 +0000 |
commit | 417ca2541aad2f4a639408da2f9d1e01acf42e08 (patch) | |
tree | 5cf13403b278867d3c7a66613bde97a283e5ec2f /automotive/vehicle/aidl/impl/utils/common/test/RecurrentTimerTest.cpp | |
parent | affc214eb8bff4e7f5ba0592be0a9ed3f36751e6 (diff) |
Avoid holding lock while calling recurrent actions.
This CL fixes a dead lock issue caused by RecurrentTimer holding
internal locks while calling actions. The dead lock is caused by
the following situation:
1. Caller holds a lock, call RecurrentTimer.registerCallback which
waits for RecurrentTimer's lock.
2. Another recurrent action happens at the same time. Recurrent
timer holds lock before calling the client action. The client action
is now waiting for the lock that is currently hold by 1.
Test: atest RecurrentTimerTest
Bug: 255574557
Change-Id: I3999f4e9cdf581cb851d5f49341dbcc0c160f234
(cherry picked from commit 93a821077effec224d3880875d98c13ef1787e4c)
Diffstat (limited to 'automotive/vehicle/aidl/impl/utils/common/test/RecurrentTimerTest.cpp')
-rw-r--r-- | automotive/vehicle/aidl/impl/utils/common/test/RecurrentTimerTest.cpp | 27 |
1 files changed, 27 insertions, 0 deletions
diff --git a/automotive/vehicle/aidl/impl/utils/common/test/RecurrentTimerTest.cpp b/automotive/vehicle/aidl/impl/utils/common/test/RecurrentTimerTest.cpp index a033a248cd..141efc135b 100644 --- a/automotive/vehicle/aidl/impl/utils/common/test/RecurrentTimerTest.cpp +++ b/automotive/vehicle/aidl/impl/utils/common/test/RecurrentTimerTest.cpp @@ -186,6 +186,33 @@ TEST_F(RecurrentTimerTest, testRegisterSameCallbackMultipleTimes) { ASSERT_EQ(countTimerCallbackQueue(&timer), static_cast<size_t>(0)); } +TEST_F(RecurrentTimerTest, testRegisterCallbackMultipleTimesNoDeadLock) { + // We want to avoid the following situation: + // Caller holds a lock while calling registerTimerCallback, registerTimerCallback will try + // to obtain an internal lock inside timer. + // Meanwhile an recurrent action happens with timer holding an internal lock. The action + // tries to obtain the lock currently hold by the caller. + // The solution is that while calling recurrent actions, timer must not hold the internal lock. + + std::unique_ptr<RecurrentTimer> timer = std::make_unique<RecurrentTimer>(); + std::mutex lock; + for (size_t i = 0; i < 1000; i++) { + std::scoped_lock<std::mutex> lockGuard(lock); + auto action = std::make_shared<RecurrentTimer::Callback>([&lock] { + // While calling this function, the timer must not hold lock in order not to dead + // lock. + std::scoped_lock<std::mutex> lockGuard(lock); + }); + // 10ms + int64_t interval = 10'000'000; + timer->registerTimerCallback(interval, action); + // Sleep for a little while to let the recurrent actions begin. + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } + // Make sure we stop the timer before we destroy lock. + timer.reset(); +} + } // namespace vehicle } // namespace automotive } // namespace hardware |