From 983ee099de60e0288997e33e3fea5c4ff647f5cd Mon Sep 17 00:00:00 2001 From: Mike Lockwood Date: Sun, 22 Nov 2009 01:42:24 -0500 Subject: [PATCH] Fix deadlock in WindowManagerService.reenableKeyguard() If reenableKeyguard() is called before the previous disableKeyguard() call is processed, then TokenWatcher.sendNotificationLocked() will cancel the request, resulting in neither the TokenWatcher acquired() or released() methods being called. In that case, reenableKeyguard() will hang waiting for released() to set mWaitingUntilKeyguardReenabled to false. Now we only wait in reenableKeyguard() if the TokenWatcher acquired() method is called and the keyguard has actually been disabled. This should fix bug b/2270192 Change-Id: Id886fb28df607dbb4543124f2db6997121d6a682 Signed-off-by: Mike Lockwood --- .../android/server/WindowManagerService.java | 42 +++++++++++-------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/services/java/com/android/server/WindowManagerService.java b/services/java/com/android/server/WindowManagerService.java index 887c46dfd9102..c009ea22c74e1 100644 --- a/services/java/com/android/server/WindowManagerService.java +++ b/services/java/com/android/server/WindowManagerService.java @@ -224,20 +224,22 @@ public class WindowManagerService extends IWindowManager.Stub /** * Condition waited on by {@link #reenableKeyguard} to know the call to * the window policy has finished. + * This is set to true only if mKeyguardTokenWatcher.acquired() has + * actually disabled the keyguard. */ - private boolean mWaitingUntilKeyguardReenabled = false; + private boolean mKeyguardDisabled = false; - - final TokenWatcher mKeyguardDisabled = new TokenWatcher( - new Handler(), "WindowManagerService.mKeyguardDisabled") { + final TokenWatcher mKeyguardTokenWatcher = new TokenWatcher( + new Handler(), "WindowManagerService.mKeyguardTokenWatcher") { public void acquired() { mPolicy.enableKeyguard(false); + mKeyguardDisabled = true; } public void released() { mPolicy.enableKeyguard(true); - synchronized (mKeyguardDisabled) { - mWaitingUntilKeyguardReenabled = false; - mKeyguardDisabled.notifyAll(); + synchronized (mKeyguardTokenWatcher) { + mKeyguardDisabled = false; + mKeyguardTokenWatcher.notifyAll(); } } }; @@ -4040,8 +4042,8 @@ public class WindowManagerService extends IWindowManager.Stub != PackageManager.PERMISSION_GRANTED) { throw new SecurityException("Requires DISABLE_KEYGUARD permission"); } - synchronized (mKeyguardDisabled) { - mKeyguardDisabled.acquire(token, tag); + synchronized (mKeyguardTokenWatcher) { + mKeyguardTokenWatcher.acquire(token, tag); } } @@ -4050,16 +4052,20 @@ public class WindowManagerService extends IWindowManager.Stub != PackageManager.PERMISSION_GRANTED) { throw new SecurityException("Requires DISABLE_KEYGUARD permission"); } - synchronized (mKeyguardDisabled) { - mKeyguardDisabled.release(token); + synchronized (mKeyguardTokenWatcher) { + mKeyguardTokenWatcher.release(token); - if (!mKeyguardDisabled.isAcquired()) { - // if we are the last one to reenable the keyguard wait until - // we have actaully finished reenabling until returning - mWaitingUntilKeyguardReenabled = true; - while (mWaitingUntilKeyguardReenabled) { + if (!mKeyguardTokenWatcher.isAcquired()) { + // If we are the last one to reenable the keyguard wait until + // we have actaully finished reenabling until returning. + // It is possible that reenableKeyguard() can be called before + // the previous disableKeyguard() is handled, in which case + // neither mKeyguardTokenWatcher.acquired() or released() would + // be called. In that case mKeyguardDisabled will be false here + // and we have nothing to wait for. + while (mKeyguardDisabled) { try { - mKeyguardDisabled.wait(); + mKeyguardTokenWatcher.wait(); } catch (InterruptedException e) { Thread.currentThread().interrupt(); } @@ -10863,7 +10869,7 @@ public class WindowManagerService extends IWindowManager.Stub public void monitor() { synchronized (mWindowMap) { } - synchronized (mKeyguardDisabled) { } + synchronized (mKeyguardTokenWatcher) { } synchronized (mKeyWaiter) { } }