Access ServiceConnector.mService in a thread-safe manner

binderDied() doesn't seem to have a defined thread on which it's called,
yet we want to abort queue processing asap on receiving it, so we have to
maintain state, modifiable from different threads.

The attached bug suggests modifying mService from a different thread is already
the case, so this just cleans up its usages to behave correctly in
parallel access scenario.

Fixed: 137744044
Test: presubmit
Change-Id: I9a09a69b5c1a1cd448a925bd7f8b2c6fa24357aa

1 files changed, 15 insertions, 10 deletions

diff --git a/core/java/com/android/internal/infra/ServiceConnector.java b/core/java/com/android/internal/infra/ServiceConnector.java
index 8136cfc09733..d6862f0188ce 100644
--- a/core/java/com/android/internal/infra/ServiceConnector.java
+++ b/core/java/com/android/internal/infra/ServiceConnector.java
@@ -228,7 +228,7 @@ public interface ServiceConnector<I extends IInterface> {
         private final int mUserId;
         private final @Nullable Function<IBinder, I> mBinderAsInterface;
 
-        private I mService = null;
+        private volatile I mService = null;
         private boolean mBinding = false;
         private boolean mUnbinding = false;
 
@@ -506,11 +506,12 @@ public interface ServiceConnector<I extends IInterface> {
 
         void unbindJobThread() {
             cancelTimeout();
-            boolean wasBound = isBound();
+            I service = mService;
+            boolean wasBound = service != null;
             if (wasBound) {
-                onServiceConnectionStatusChanged(mService, false);
+                onServiceConnectionStatusChanged(service, false);
                 mContext.unbindService(mServiceConnection);
-                mService.asBinder().unlinkToDeath(this, 0);
+                service.asBinder().unlinkToDeath(this, 0);
                 mService = null;
             }
             mBinding = false;
@@ -543,7 +544,7 @@ public interface ServiceConnector<I extends IInterface> {
         }
 
         @Override
-        public void onServiceConnected(@NonNull ComponentName name, @NonNull IBinder service) {
+        public void onServiceConnected(@NonNull ComponentName name, @NonNull IBinder binder) {
             if (mUnbinding) {
                 Log.i(LOG_TAG, "Ignoring onServiceConnected due to ongoing unbinding: " + this);
                 return;
@@ -551,14 +552,15 @@ public interface ServiceConnector<I extends IInterface> {
             if (DEBUG) {
                 logTrace();
             }
-            mService = binderAsInterface(service);
+            I service = binderAsInterface(binder);
+            mService = service;
             mBinding = false;
             try {
-                service.linkToDeath(ServiceConnector.Impl.this, 0);
+                binder.linkToDeath(ServiceConnector.Impl.this, 0);
             } catch (RemoteException e) {
                 Log.e(LOG_TAG, "onServiceConnected " + name + ": ", e);
             }
-            onServiceConnectionStatusChanged(mService, true);
+            onServiceConnectionStatusChanged(service, true);
             processQueue();
         }
 
@@ -568,8 +570,11 @@ public interface ServiceConnector<I extends IInterface> {
                 logTrace();
             }
             mBinding = true;
-            onServiceConnectionStatusChanged(mService, false);
-            mService = null;
+            I service = mService;
+            if (service != null) {
+                onServiceConnectionStatusChanged(service, false);
+                mService = null;
+            }
         }
 
         @Override