diff options
| author | 2020-06-19 10:38:02 +0800 | |
|---|---|---|
| committer | 2020-06-19 12:09:54 +0800 | |
| commit | c9f0c232ae6247af96a38776965c3c7a5d27bef2 (patch) | |
| tree | 574ace57b359cb235dea16284e50cb61869df07f | |
| parent | 4fdd3c8e34bfd675f7be3cf74faf3d6020737acf (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.java | 72 |
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 |