From 852d9fec6d5febc60641dfac60ea4b8f145f5b2f Mon Sep 17 00:00:00 2001 From: "Zhang, GaofengX" Date: Tue, 11 Jun 2019 11:16:30 +0800 Subject: Avoid key_queue_mutex deadlock in waitkey() Waitkey() is designed to obtain lock "key_queue_mutex" in the very beginning of function. int RecoveryUI::WaitKey() { std::unique_lock lk(key_queue_mutex); ... } However, there's case "key_queue_mutex" being applied again in waitkey(), thus cause deadlock. There are two reproduce scenario: 1.Executing "fastboot reboot recovery" in userspace fastboot 2.Executing "adb reboot fastboot" in recovery os When entering userspace fastboot/recovery, waitkey() will wait there for user action. fastboot/adb commands will trigger ui->interruptkey() to notify the thread waitkey() in. In the next, waitkey() will move on and call SetScreenSaveState(), which do LOG(ERROR) in fail case of brightness set. LOG(ERROR) is designed to print log on UI. Unfortunately, UI->print() applies lock "key_queue_mutex" too, so deadlock happen. Note: Here is details how lock "key_queue_mutex" applied in UI->print(): Function Print() call Function PrintV() call Function update_screen_locked() call Function draw_screen_locked() call Function draw_menu_and_test_buffer_locked() call Function IsLongPress() bool RecoveryUI::IsLongPress() { std::lock_guard lg(key_queue_mutex); bool result = key_long_press; return result; } Bug: 135078366 Test: no errors when running "fastboot reboot recovery" in userspace fastboot & "adb reboot fastboot" in recovery os Change-Id: Ida6b3c4ba9896a70021373f02a94954f0a60cf31 Signed-off-by: Zhang, GaofengX Signed-off-by: Xihua Chen --- recovery_ui/ui.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/recovery_ui/ui.cpp b/recovery_ui/ui.cpp index b7107ff21..f1a79dcfd 100644 --- a/recovery_ui/ui.cpp +++ b/recovery_ui/ui.cpp @@ -419,7 +419,7 @@ void RecoveryUI::SetScreensaverState(ScreensaverState state) { LOG(INFO) << "Brightness: " << brightness_normal_value_ << " (" << brightness_normal_ << "%)"; } else { - LOG(ERROR) << "Unable to set brightness to normal"; + LOG(WARNING) << "Unable to set brightness to normal"; } break; case ScreensaverState::DIMMED: @@ -429,7 +429,7 @@ void RecoveryUI::SetScreensaverState(ScreensaverState state) { << "%)"; screensaver_state_ = ScreensaverState::DIMMED; } else { - LOG(ERROR) << "Unable to set brightness to dim"; + LOG(WARNING) << "Unable to set brightness to dim"; } break; case ScreensaverState::OFF: @@ -437,7 +437,7 @@ void RecoveryUI::SetScreensaverState(ScreensaverState state) { LOG(INFO) << "Brightness: 0 (off)"; screensaver_state_ = ScreensaverState::OFF; } else { - LOG(ERROR) << "Unable to set brightness to off"; + LOG(WARNING) << "Unable to set brightness to off"; } break; default: -- cgit v1.2.3