Commit 8f9c68fa authored Aug 07, 2024 by Lee Shombert

Correct potential deadlock in AnrTimerService

This change eliminates a potential deadlock that would occur if an ANR
timer expires but the upper Java layer has lost track of the timer.
(The mechanism by which the upper layer loses track of the timer is
not understood.)  If the upper layer cannot handle the timer
expiration, the native layer cleans up its records.  That clean-up was
locking a mutex and then calling AnrTimerService::discard, which takes
the mutex again.  Mutexes are non-recursive and this is a deadlock.

The fix removes the extraneous lock.  A dangling comment is also
cleaned up.

Tested with a custom build that triggered the deadlock condition.  The
baseline code generated an ANR report that matches the reports in the
bug.  The modified code did not generate an ANR and proceeded normally.

Flag: EXEMPT bugfix
Bug: 357341347
Test: atest
 * FrameworksServicesTests:AnrTimerTest
 * FrameworksServicesTests:com.android.server.am
 * FrameworksMockingServicesTests:com.android.server.am
Change-Id: I12aeef5905409dc6c062e1fbd23a000cf0c9515c

parent cd0e636d

services/core/jni/com_android_server_utils_AnrTimer.cpp

+0 −2

Original line number	Diff line number	Diff line
		@@ -934,7 +934,6 @@ void AnrTimerService::scrubExpiredLocked() {
		}

		// Hold the lock in order to manage the running list.
		// the listener.
		void AnrTimerService::expire(timer_id_t timerId) {
		// Save the timer attributes for the notification
		int pid = 0;
		@@ -967,7 +966,6 @@ void AnrTimerService::expire(timer_id_t timerId) {
		// Deliver the notification outside of the lock.
		if (expired) {
		if (!notifier_(timerId, pid, uid, elapsed, notifierCookie_, notifierObject_)) {
		AutoMutex _l(lock_);
		// Notification failed, which means the listener will never call accept() or
		// discard(). Do not reinsert the timer.
		discard(timerId);