From 705b6af1b0ba884d780bb586c1127b594e08e7e6 Mon Sep 17 00:00:00 2001
From: Soonil Nagarkar <sooniln@google.com>
Date: Tue, 10 Mar 2020 17:27:50 -0700
Subject: [PATCH] Fix disconnection bug in ServiceWatcher

Context.unbind() does not result in any callback to
onServiceDisconnected(), even though that might be expected. Do the
callback ourselves if necessary to ensure that clients always see
disconnections.

Bug: 151182828
Test: manual
Change-Id: I4969d4f548a6ff7256a4833d61cdd1263ca1dfde
---
 .../com/android/server/ServiceWatcher.java    | 24 ++++++++-----------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/services/core/java/com/android/server/ServiceWatcher.java b/services/core/java/com/android/server/ServiceWatcher.java
index b43ae36c7ef58..cfb79aa3a210e 100644
--- a/services/core/java/com/android/server/ServiceWatcher.java
+++ b/services/core/java/com/android/server/ServiceWatcher.java
@@ -100,7 +100,7 @@ public class ServiceWatcher implements ServiceConnection {
         @Nullable public final ComponentName component;
         @UserIdInt public final int userId;
 
-        private ServiceInfo(ResolveInfo resolveInfo, int currentUserId) {
+        ServiceInfo(ResolveInfo resolveInfo, int currentUserId) {
             Preconditions.checkArgument(resolveInfo.serviceInfo.getComponentName() != null);
 
             Bundle metadata = resolveInfo.serviceInfo.metaData;
@@ -316,6 +316,7 @@ public class ServiceWatcher implements ServiceConnection {
             }
 
             mContext.unbindService(this);
+            onServiceDisconnected(mServiceInfo.component);
             mServiceInfo = ServiceInfo.NONE;
         }
 
@@ -339,15 +340,13 @@ public class ServiceWatcher implements ServiceConnection {
     @Override
     public final void onServiceConnected(ComponentName component, IBinder binder) {
         Preconditions.checkState(Looper.myLooper() == mHandler.getLooper());
+        Preconditions.checkState(mBinder == null);
 
         if (D) {
             Log.i(TAG, getLogPrefix() + " connected to " + component.toShortString());
         }
 
         mBinder = binder;
-
-        // we always run the on bind callback even if we know that the binder is dead already so
-        // that there are always balance pairs of bind/unbind callbacks
         if (mOnBind != null) {
             try {
                 mOnBind.run(binder);
@@ -357,19 +356,16 @@ public class ServiceWatcher implements ServiceConnection {
                 Log.e(TAG, getLogPrefix() + " exception running on " + mServiceInfo, e);
             }
         }
-
-        try {
-            // setting the binder to null lets us skip queued transactions
-            binder.linkToDeath(() -> mBinder = null, 0);
-        } catch (RemoteException e) {
-            mBinder = null;
-        }
     }
 
     @Override
     public final void onServiceDisconnected(ComponentName component) {
         Preconditions.checkState(Looper.myLooper() == mHandler.getLooper());
 
+        if (mBinder == null) {
+            return;
+        }
+
         if (D) {
             Log.i(TAG, getLogPrefix() + " disconnected from " + component.toShortString());
         }
@@ -391,18 +387,18 @@ public class ServiceWatcher implements ServiceConnection {
         onBestServiceChanged(true);
     }
 
-    private void onUserSwitched(@UserIdInt int userId) {
+    void onUserSwitched(@UserIdInt int userId) {
         mCurrentUserId = userId;
         onBestServiceChanged(false);
     }
 
-    private void onUserUnlocked(@UserIdInt int userId) {
+    void onUserUnlocked(@UserIdInt int userId) {
         if (userId == mCurrentUserId) {
             onBestServiceChanged(false);
         }
     }
 
-    private void onPackageChanged(String packageName) {
+    void onPackageChanged(String packageName) {
         // force a rebind if the changed package was the currently connected package
         String currentPackageName =
                 mServiceInfo.component != null ? mServiceInfo.component.getPackageName() : null;