Fix Race condition when shutting down executor/transport channel

dependencies.properties

@@ -76,5 +76,6 @@ maven.junit_junit=junit:junit:4.12
 maven.org_mockito_mockito_core=org.mockito:mockito-core:2.21.0
 maven.org_hamcrest_hamcrest_core=org.hamcrest:hamcrest-core:1.3
 maven.com_google_truth_truth=com.google.truth:truth:0.44
+maven.com_googlecode_java_diff_utils_diffutils=com.googlecode.java-diff-utils:diffutils:1.3.0
 maven.net_bytebuddy_byte_buddy=net.bytebuddy:byte-buddy:1.8.15
 maven.org_objenesis_objenesis=org.objenesis:objenesis:2.6

gax-grpc/BUILD.bazel

@@ -37,6 +37,7 @@ _TEST_COMPILE_DEPS = [
     "@com_google_api_grpc_grpc_google_common_protos//jar",
     "@org_apache_commons_commons_lang3//jar",
     "//gax:gax_testlib",
+    "@com_googlecode_java_diff_utils_diffutils//jar",
 ]
 
 java_library(

gax-httpjson/BUILD.bazel

@@ -26,6 +26,7 @@ _TEST_COMPILE_DEPS = [
     "@org_mockito_mockito_core//jar",
     "@com_google_truth_truth//jar",
     "//gax:gax_testlib",
+   "@com_googlecode_java_diff_utils_diffutils//jar",
 ]
 
 java_library(

gax/BUILD.bazel

@@ -31,6 +31,7 @@ _TEST_COMPILE_DEPS = [
     "@org_hamcrest_hamcrest_core//jar",
     "@net_bytebuddy_byte_buddy//jar",
     "@org_objenesis_objenesis//jar",
+    "@com_googlecode_java_diff_utils_diffutils//jar",
 ]
 
 java_library(

gax/src/main/java/com/google/api/gax/rpc/ClientContext.java

@@ -127,15 +127,10 @@ public static ClientContext create(ClientSettings settings) throws IOException {
    * settings.
    */
   public static ClientContext create(StubSettings settings) throws IOException {
-    ImmutableList.Builder<BackgroundResource> backgroundResources = ImmutableList.builder();
-
     ApiClock clock = settings.getClock();
 
     ExecutorProvider executorProvider = settings.getExecutorProvider();
     final ScheduledExecutorService executor = executorProvider.getExecutor();
-    if (executorProvider.shouldAutoClose()) {
-      backgroundResources.add(new ExecutorAsBackgroundResource(executor));
-    }
 
     Credentials credentials = settings.getCredentialsProvider().getCredentials();
 
@@ -158,9 +153,6 @@ public static ClientContext create(StubSettings settings) throws IOException {
       transportChannelProvider = transportChannelProvider.withCredentials(credentials);
     }
     TransportChannel transportChannel = transportChannelProvider.getTransportChannel();
-    if (transportChannelProvider.shouldAutoClose()) {
-      backgroundResources.add(transportChannel);
-    }
 
     ApiCallContext defaultCallContext =
         transportChannel.getEmptyCallContext().withTransportChannel(transportChannel);
@@ -185,6 +177,15 @@ public static ClientContext create(StubSettings settings) throws IOException {
       watchdog = watchdogProvider.getWatchdog();
     }
 
+    ImmutableList.Builder<BackgroundResource> backgroundResources = ImmutableList.builder();
+
+    if (transportChannelProvider.shouldAutoClose()) {
+      backgroundResources.add(transportChannel);
+    }
+    if (executorProvider.shouldAutoClose()) {
+      backgroundResources.add(new ExecutorAsBackgroundResource(executor));
+    }
+
     return newBuilder()
         .setBackgroundResources(backgroundResources.build())
         .setExecutor(executor)

gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java

@@ -40,6 +40,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.truth.Truth;
 import java.io.IOException;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
@@ -270,11 +271,20 @@ private void runTest(
     Truth.assertThat(executor.shutdownCalled).isFalse();
     Truth.assertThat(transportChannel.isShutdown()).isFalse();
 
-    for (BackgroundResource backgroundResource : clientContext.getBackgroundResources()) {
-      backgroundResource.shutdown();
+    List<BackgroundResource> resources = clientContext.getBackgroundResources();
+
+    if (!resources.isEmpty()) {
+      // This is slightly too implementation-specific, but we need to ensure that executor is shut
+      // down after the transportChannel: https://github.com/googleapis/gax-java/issues/785
+      Truth.assertThat(resources.size()).isEqualTo(2);
+      Truth.assertThat(transportChannel.isShutdown()).isNotEqualTo(shouldAutoClose);
+      Truth.assertThat(executor.shutdownCalled).isNotEqualTo(shouldAutoClose);
+      resources.get(0).shutdown();
+      Truth.assertThat(transportChannel.isShutdown()).isEqualTo(shouldAutoClose);
+      Truth.assertThat(executor.shutdownCalled).isNotEqualTo(shouldAutoClose);
+      resources.get(1).shutdown();
+      Truth.assertThat(transportChannel.isShutdown()).isEqualTo(shouldAutoClose);
+      Truth.assertThat(executor.shutdownCalled).isEqualTo(shouldAutoClose);
     }
-
-    Truth.assertThat(executor.shutdownCalled).isEqualTo(shouldAutoClose);
-    Truth.assertThat(transportChannel.isShutdown()).isEqualTo(shouldAutoClose);
   }
 }