Don't deadlock during shutdown() (#3374)

Author: schmidt-sebastian

firebase-firestore/CHANGELOG.md

@@ -4,8 +4,10 @@ by opting into a release at
 
 # 24.0.2
 - [fixed] Fixed an AppCheck issue that caused Firestore listeners to stop
-working and receive a "Permission Denied" error. This issue only occurred for
-AppCheck users that set their expiration time to under an hour.
+  working and receive a "Permission Denied" error. This issue only occurred for
+  AppCheck users that set their expiration time to under an hour.
+- [fixed] Fixed a potential problem during Firestore's shutdown that prevented 
+  the shutdown from proceeding if a network connection was opened right before.
 
 # 24.0.1
 - [changed] Improved performance for databases that contain many document

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/FirestoreCallCredentials.java

@@ -19,6 +19,7 @@
 import com.google.firebase.FirebaseApiNotAvailableException;
 import com.google.firebase.firestore.auth.CredentialsProvider;
 import com.google.firebase.firestore.auth.User;
+import com.google.firebase.firestore.util.Executors;
 import com.google.firebase.firestore.util.Logger;
 import com.google.firebase.internal.api.FirebaseNoSignedInUserException;
 import io.grpc.CallCredentials;
@@ -58,7 +59,12 @@ public void applyRequestMetadata(
 
     Tasks.whenAll(authTask, appCheckTask)
         .addOnCompleteListener(
-            executor,
+            // We previously used the executor that is passed to us by the callee of the method
+            // (which happens to be the AsyncQueue). This sometimes led to deadlocks during
+            // shutdown, as Firestore's termiante() runs on the AsyncQueue, which would then invoke
+            // GRPC's shutdown(), which would then wait for this callback to be scheduled on the
+            // AsyncQueue.
+            Executors.DIRECT_EXECUTOR,
             unused -> {
               Metadata metadata = new Metadata();
 

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/GrpcCallProvider.java

@@ -15,7 +15,6 @@
 package com.google.firebase.firestore.remote;
 
 import android.content.Context;
-import androidx.annotation.VisibleForTesting;
 import com.google.android.gms.common.GooglePlayServicesNotAvailableException;
 import com.google.android.gms.common.GooglePlayServicesRepairableException;
 import com.google.android.gms.security.ProviderInstaller;
@@ -64,19 +63,6 @@ public class GrpcCallProvider {
   private final DatabaseInfo databaseInfo;
   private final CallCredentials firestoreHeaders;
 
-  /**
-   * Helper function to globally override the channel that RPCs use. Useful for testing when you
-   * want to bypass SSL certificate checking.
-   *
-   * @param channelBuilderSupplier The supplier for a channel builder that is used to create gRPC
-   *     channels.
-   */
-  @VisibleForTesting
-  public static void overrideChannelBuilder(
-      Supplier<ManagedChannelBuilder<?>> channelBuilderSupplier) {
-    overrideChannelBuilderSupplier = channelBuilderSupplier;
-  }
-
   GrpcCallProvider(
       AsyncQueue asyncQueue,
       Context context,
@@ -142,7 +128,7 @@ <ReqT, RespT> Task<ClientCall<ReqT, RespT>> createClientCall(
   void shutdown() {
     // Handling shutdown synchronously to avoid re-enqueuing on the AsyncQueue after shutdown has
     // started.
-    ManagedChannel channel = null;
+    ManagedChannel channel;
     try {
       channel = Tasks.await(channelTask);
     } catch (ExecutionException e) {