From 417ca2541aad2f4a639408da2f9d1e01acf42e08 Mon Sep 17 00:00:00 2001 From: Yu Shan Date: Thu, 8 Dec 2022 11:39:26 -0800 Subject: 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) --- .../impl/utils/common/test/RecurrentTimerTest.cpp | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'automotive/vehicle/aidl/impl/utils/common/test/RecurrentTimerTest.cpp') 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(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 timer = std::make_unique(); + std::mutex lock; + for (size_t i = 0; i < 1000; i++) { + std::scoped_lock lockGuard(lock); + auto action = std::make_shared([&lock] { + // While calling this function, the timer must not hold lock in order not to dead + // lock. + std::scoped_lock 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 -- cgit v1.2.3