binder: Avoid potential deadlock when canceling AsyncSecurityPolicy futures (#12283)

Move future cancellation outside of synchronized block in
`BinderClientTransport.notifyTerminated()` to prevent deadlock if
`AsyncSecurityPolicy` uses `directExecutor()` for callbacks.

Fixes #12190

---------

Signed-off-by: Hyunsang Han <gustkd3@gmail.com>

Author: HyunSangHan

binder/src/main/java/io/grpc/binder/internal/BinderClientTransport.java

@@ -87,13 +87,6 @@ public final class BinderClientTransport extends BinderTransport
   @GuardedBy("this")
   private ScheduledFuture<?> readyTimeoutFuture; // != null iff timeout scheduled.
 
-  @GuardedBy("this")
-  @Nullable
-  private ListenableFuture<Status> authResultFuture; // null before we check auth.
-
-  @GuardedBy("this")
-  @Nullable
-  private ListenableFuture<Status> preAuthResultFuture; // null before we pre-auth.
 
   /**
    * Constructs a new transport instance.
@@ -193,7 +186,8 @@ private void preAuthorize(ServiceInfo serviceInfo) {
     // unauthorized server a chance to run, but the connection will still fail by SecurityPolicy
     // check later in handshake. Pre-auth remains effective at mitigating abuse because malware
     // can't typically control the exact timing of its installation.
-    preAuthResultFuture = checkServerAuthorizationAsync(serviceInfo.applicationInfo.uid);
+    ListenableFuture<Status> preAuthResultFuture =
+        register(checkServerAuthorizationAsync(serviceInfo.applicationInfo.uid));
     Futures.addCallback(
         preAuthResultFuture,
         new FutureCallback<Status>() {
@@ -314,12 +308,6 @@ void notifyTerminated() {
       readyTimeoutFuture.cancel(false);
       readyTimeoutFuture = null;
     }
-    if (preAuthResultFuture != null) {
-      preAuthResultFuture.cancel(false); // No effect if already complete.
-    }
-    if (authResultFuture != null) {
-      authResultFuture.cancel(false); // No effect if already complete.
-    }
     serviceBinding.unbind();
     clientTransportListener.transportTerminated();
   }
@@ -339,7 +327,8 @@ protected void handleSetupTransport(Parcel parcel) {
       } else {
         restrictIncomingBinderToCallsFrom(remoteUid);
         attributes = setSecurityAttrs(attributes, remoteUid);
-        authResultFuture = checkServerAuthorizationAsync(remoteUid);
+        ListenableFuture<Status> authResultFuture =
+            register(checkServerAuthorizationAsync(remoteUid));
         Futures.addCallback(
             authResultFuture,
             new FutureCallback<Status>() {
@@ -398,6 +387,7 @@ protected void handlePingResponse(Parcel parcel) {
     pingTracker.onPingResponse(parcel.readInt());
   }
 
+
   private static ClientStream newFailingClientStream(
       Status failure, Attributes attributes, Metadata headers, ClientStreamTracer[] tracers) {
     StatsTraceContext statsTraceContext =

binder/src/main/java/io/grpc/binder/internal/BinderTransport.java

@@ -45,9 +45,11 @@
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -166,6 +168,9 @@ protected enum TransportState {
   @GuardedBy("this")
   private final LinkedHashSet<Integer> callIdsToNotifyWhenReady = new LinkedHashSet<>();
 
+  @GuardedBy("this")
+  private final List<Future<?>> ownedFutures = new ArrayList<>(); // To cancel upon terminate.
+
   @GuardedBy("this")
   protected Attributes attributes;
 
@@ -249,6 +254,13 @@ void releaseExecutors() {
     executorServicePool.returnObject(scheduledExecutorService);
   }
 
+  // Registers the specified future for eventual safe cancellation upon shutdown/terminate.
+  @GuardedBy("this")
+  protected final <T extends Future<?>> T register(T future) {
+    ownedFutures.add(future);
+    return future;
+  }
+
   @GuardedBy("this")
   boolean inState(TransportState transportState) {
     return this.transportState == transportState;
@@ -299,6 +311,8 @@ final void shutdownInternal(Status shutdownStatus, boolean forceTerminate) {
       sendShutdownTransaction();
       ArrayList<Inbound<?>> calls = new ArrayList<>(ongoingCalls.values());
       ongoingCalls.clear();
+      ArrayList<Future<?>> futuresToCancel = new ArrayList<>(ownedFutures);
+      ownedFutures.clear();
       scheduledExecutorService.execute(
           () -> {
             for (Inbound<?> inbound : calls) {
@@ -310,6 +324,11 @@ final void shutdownInternal(Status shutdownStatus, boolean forceTerminate) {
               notifyTerminated();
             }
             releaseExecutors();
+            
+            for (Future<?> future : futuresToCancel) {
+              // Not holding any locks here just in case some listener runs on a direct Executor.
+              future.cancel(false); // No effect if already isDone().
+            }
           });
     }
   }