From 4621a0c928a26004191b79d3bd4dccf7f1e1cccb Mon Sep 17 00:00:00 2001 From: Joanne Chung Date: Thu, 6 Apr 2023 20:38:46 +0800 Subject: [PATCH] Fix system crash due to GREF overflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TCMS uses LruCache to save sessions, it will handle unlink DeathRecipient when session is destroyed. But the TCMS doesn’t handle unlink DeathRecipient if there are many sessions that exceed the cache size. In this change, the system will unlink DeathRecipient if the session is evicted from LruCache. But this change doesn't handle the behavior if the elements are evicted from the cache. This is unrelated to the crash, it will be handled later. Bug: 275298870 Test: local update cache size to 1, use testMultipleActiveSessions() to verify the evicted behavior. Change-Id: Ie0f23b26005c73080473a1550ef7d1f275bf7e27 --- .../TextClassificationManagerService.java | 82 +++++++++++++------ 1 file changed, 55 insertions(+), 27 deletions(-) diff --git a/services/core/java/com/android/server/textclassifier/TextClassificationManagerService.java b/services/core/java/com/android/server/textclassifier/TextClassificationManagerService.java index 984cb19cbbf99..31348cd9156f5 100644 --- a/services/core/java/com/android/server/textclassifier/TextClassificationManagerService.java +++ b/services/core/java/com/android/server/textclassifier/TextClassificationManagerService.java @@ -42,7 +42,6 @@ import android.service.textclassifier.ITextClassifierService; import android.service.textclassifier.TextClassifierService; import android.service.textclassifier.TextClassifierService.ConnectionState; import android.text.TextUtils; -import android.util.ArrayMap; import android.util.LruCache; import android.util.Slog; import android.util.SparseArray; @@ -74,7 +73,7 @@ import java.io.FileDescriptor; import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; -import java.util.Map; +import java.util.NoSuchElementException; import java.util.Objects; /** @@ -388,14 +387,13 @@ public final class TextClassificationManagerService extends ITextClassifierServi synchronized (mLock) { final StrippedTextClassificationContext textClassificationContext = - mSessionCache.get(sessionId); + mSessionCache.get(sessionId.getToken()); final int userId = textClassificationContext != null ? textClassificationContext.userId : UserHandle.getCallingUserId(); final boolean useDefaultTextClassifier = - textClassificationContext != null - ? textClassificationContext.useDefaultTextClassifier - : true; + textClassificationContext == null + || textClassificationContext.useDefaultTextClassifier; final SystemTextClassifierMetadata sysTcMetadata = new SystemTextClassifierMetadata( "", userId, useDefaultTextClassifier); @@ -405,7 +403,7 @@ public final class TextClassificationManagerService extends ITextClassifierServi /* attemptToBind= */ false, service -> { service.onDestroyTextClassificationSession(sessionId); - mSessionCache.remove(sessionId); + mSessionCache.remove(sessionId.getToken()); }, "onDestroyTextClassificationSession", NO_OP_CALLBACK); @@ -676,14 +674,39 @@ public final class TextClassificationManagerService extends ITextClassifierServi @NonNull private final Object mLock; + + @NonNull + private final DeathRecipient mDeathRecipient = new DeathRecipient() { + @Override + public void binderDied() { + // no-op + } + + @Override + public void binderDied(IBinder who) { + if (DEBUG) { + Slog.d(LOG_TAG, "binderDied for " + who); + } + remove(who); + } + }; @NonNull @GuardedBy("mLock") - private final LruCache - mCache = new LruCache<>(MAX_CACHE_SIZE); - @NonNull - @GuardedBy("mLock") - private final Map mDeathRecipients = - new ArrayMap<>(); + private final LruCache + mCache = new LruCache<>(MAX_CACHE_SIZE) { + @Override + protected void entryRemoved(boolean evicted, + IBinder token, + StrippedTextClassificationContext oldValue, + StrippedTextClassificationContext newValue) { + if (evicted) { + // The remove(K) or put(K, V) should be handled + token.unlinkToDeath(mDeathRecipient, /* flags= */ 0); + // TODO(b/278160706): handle app process and TCS's behavior if the + // session is removed by system server + } + } + }; SessionCache(@NonNull Object lock) { mLock = Objects.requireNonNull(lock); @@ -692,12 +715,10 @@ public final class TextClassificationManagerService extends ITextClassifierServi void put(@NonNull TextClassificationSessionId sessionId, @NonNull TextClassificationContext textClassificationContext) { synchronized (mLock) { - mCache.put(sessionId, + mCache.put(sessionId.getToken(), new StrippedTextClassificationContext(textClassificationContext)); try { - DeathRecipient deathRecipient = () -> remove(sessionId); - sessionId.getToken().linkToDeath(deathRecipient, /* flags= */ 0); - mDeathRecipients.put(sessionId, deathRecipient); + sessionId.getToken().linkToDeath(mDeathRecipient, /* flags= */ 0); } catch (RemoteException e) { Slog.w(LOG_TAG, "SessionCache: Failed to link to death", e); } @@ -705,22 +726,29 @@ public final class TextClassificationManagerService extends ITextClassifierServi } @Nullable - StrippedTextClassificationContext get(@NonNull TextClassificationSessionId sessionId) { - Objects.requireNonNull(sessionId); + StrippedTextClassificationContext get(@NonNull IBinder token) { + Objects.requireNonNull(token); synchronized (mLock) { - return mCache.get(sessionId); + return mCache.get(token); } } - void remove(@NonNull TextClassificationSessionId sessionId) { - Objects.requireNonNull(sessionId); + void remove(@NonNull IBinder token) { + Objects.requireNonNull(token); synchronized (mLock) { - DeathRecipient deathRecipient = mDeathRecipients.get(sessionId); - if (deathRecipient != null) { - sessionId.getToken().unlinkToDeath(deathRecipient, /* flags= */ 0); + if (DEBUG) { + Slog.d(LOG_TAG, "SessionCache: remove for " + token); } - mDeathRecipients.remove(sessionId); - mCache.remove(sessionId); + if (token != null) { + try { + token.unlinkToDeath(mDeathRecipient, /* flags= */ 0); + } catch (NoSuchElementException e) { + if (DEBUG) { + Slog.d(LOG_TAG, "SessionCache: " + token + " was already unlinked."); + } + } + } + mCache.remove(token); } }