summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author TYM Tsai <tymtsai@google.com> 2020-06-19 10:38:02 +0800
committer TYM Tsai <tymtsai@google.com> 2020-06-19 12:09:54 +0800
commitc9f0c232ae6247af96a38776965c3c7a5d27bef2 (patch)
tree574ace57b359cb235dea16284e50cb61869df07f
parent4fdd3c8e34bfd675f7be3cf74faf3d6020737acf (diff)
Fix the context leak in the ContentCapture
Activity cannot be released because the context be holded by multiple objects. * Activity holds ContentCaptureManager. * ContentCaptureManager holds the context and MainContextCaptureSession. * MainContextCaptureSession holds the context and ContentCaptureManager. * The system server holds some binder references to MainContentCaptureSession. If the system service never released the binder references, then the activity is also never GC'd. To avoid the issue, 1. Make the session state receiver of MainContextCaptureSession to be static and uses weak reference to MainContextCaptureSession. 2. The direct service vulture may miss to do unlinkToDeath(), do a checking while the session destory. Bug: 143210612 Test: manual check Objects on the meminfo Test: Activities should be released after a period of time Test: adb shell dumpsys meminfo com.google.android.dialer Change-Id: I12037483addb1efe444c74fa189ef6afd15821dd
-rw-r--r--core/java/android/view/contentcapture/MainContentCaptureSession.java72
1 files changed, 45 insertions, 27 deletions
diff --git a/core/java/android/view/contentcapture/MainContentCaptureSession.java b/core/java/android/view/contentcapture/MainContentCaptureSession.java
index 6eb71f747be6..c43beea98c7e 100644
--- a/core/java/android/view/contentcapture/MainContentCaptureSession.java
+++ b/core/java/android/view/contentcapture/MainContentCaptureSession.java
@@ -52,6 +52,7 @@ import android.view.contentcapture.ViewNode.ViewStructureImpl;
import com.android.internal.os.IResultReceiver;
import java.io.PrintWriter;
+import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -146,7 +147,44 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
* Binder object used to update the session state.
*/
@NonNull
- private final IResultReceiver.Stub mSessionStateReceiver;
+ private final SessionStateReceiver mSessionStateReceiver;
+
+ private static class SessionStateReceiver extends IResultReceiver.Stub {
+ private final WeakReference<MainContentCaptureSession> mMainSession;
+
+ SessionStateReceiver(MainContentCaptureSession session) {
+ mMainSession = new WeakReference<>(session);
+ }
+
+ @Override
+ public void send(int resultCode, Bundle resultData) {
+ final MainContentCaptureSession mainSession = mMainSession.get();
+ if (mainSession == null) {
+ Log.w(TAG, "received result after mina session released");
+ return;
+ }
+ final IBinder binder;
+ if (resultData != null) {
+ // Change in content capture enabled.
+ final boolean hasEnabled = resultData.getBoolean(EXTRA_ENABLED_STATE);
+ if (hasEnabled) {
+ final boolean disabled = (resultCode == RESULT_CODE_FALSE);
+ mainSession.mDisabled.set(disabled);
+ return;
+ }
+ binder = resultData.getBinder(EXTRA_BINDER);
+ if (binder == null) {
+ Log.wtf(TAG, "No " + EXTRA_BINDER + " extra result");
+ mainSession.mHandler.post(() -> mainSession.resetSession(
+ STATE_DISABLED | STATE_INTERNAL_ERROR));
+ return;
+ }
+ } else {
+ binder = null;
+ }
+ mainSession.mHandler.post(() -> mainSession.onSessionStarted(resultCode, binder));
+ }
+ }
protected MainContentCaptureSession(@NonNull Context context,
@NonNull ContentCaptureManager manager, @NonNull Handler handler,
@@ -159,32 +197,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
final int logHistorySize = mManager.mOptions.logHistorySize;
mFlushHistory = logHistorySize > 0 ? new LocalLog(logHistorySize) : null;
- mSessionStateReceiver = new IResultReceiver.Stub() {
- @Override
- public void send(int resultCode, Bundle resultData) {
- final IBinder binder;
- if (resultData != null) {
- // Change in content capture enabled.
- final boolean hasEnabled = resultData.getBoolean(EXTRA_ENABLED_STATE);
- if (hasEnabled) {
- final boolean disabled = (resultCode == RESULT_CODE_FALSE);
- mDisabled.set(disabled);
- return;
- }
- binder = resultData.getBinder(EXTRA_BINDER);
- if (binder == null) {
- Log.wtf(TAG, "No " + EXTRA_BINDER + " extra result");
- mHandler.post(() -> resetSession(
- STATE_DISABLED | STATE_INTERNAL_ERROR));
- return;
- }
- } else {
- binder = null;
- }
- mHandler.post(() -> onSessionStarted(resultCode, binder));
- }
- };
-
+ mSessionStateReceiver = new SessionStateReceiver(this);
}
@Override
@@ -543,6 +556,11 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
Log.e(TAG, "Error destroying system-service session " + mId + " for "
+ getDebugState() + ": " + e);
}
+
+ if (mDirectServiceInterface != null) {
+ mDirectServiceInterface.asBinder().unlinkToDeath(mDirectServiceVulture, 0);
+ }
+ mDirectServiceInterface = null;
}
// TODO(b/122454205): once we support multiple sessions, we might need to move some of these