diff --git a/dependencies.properties b/dependencies.properties index bba29b780..0a000e652 100644 --- a/dependencies.properties +++ b/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 diff --git a/gax-grpc/BUILD.bazel b/gax-grpc/BUILD.bazel index 2ec5b4b77..46a322255 100644 --- a/gax-grpc/BUILD.bazel +++ b/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( diff --git a/gax-httpjson/BUILD.bazel b/gax-httpjson/BUILD.bazel index 8037511bd..bba76e8ac 100644 --- a/gax-httpjson/BUILD.bazel +++ b/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( diff --git a/gax/BUILD.bazel b/gax/BUILD.bazel index e93a0e898..ef99df10e 100644 --- a/gax/BUILD.bazel +++ b/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( diff --git a/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index c5471b04c..391f3d959 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/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 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 backgroundResources = ImmutableList.builder(); + + if (transportChannelProvider.shouldAutoClose()) { + backgroundResources.add(transportChannel); + } + if (executorProvider.shouldAutoClose()) { + backgroundResources.add(new ExecutorAsBackgroundResource(executor)); + } + return newBuilder() .setBackgroundResources(backgroundResources.build()) .setExecutor(executor) diff --git a/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index 1eadb8de4..2d0830a5e 100644 --- a/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/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 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); } }