fix: prevent hanging by call backgroundResources.close() on stub.close() [ggj] (#804)

miraleung · web-flow · commit 428db97c1534 · 2021-08-02T15:24:17.000-07:00
* feat(ast): Add support for throwable causes

* fix: call backgroundResources.close() on stub.close()

* fix: make Throwable a static final in TypeNode

* fix: update goldens

* feat(ast): support throwing all kinds of expressions

* fix: call backgroundResources.close() on stub.close()

* fix: update goldens

* feat(ast): Add support for multi-catch blocks

* fix: add extra catch block

* fix: isolate stub.close change to another PR

* fix: merge branches
diff --git a/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceStubClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceStubClassComposer.java
@@ -40,6 +40,8 @@
 import com.google.api.generator.engine.ast.ScopeNode;
 import com.google.api.generator.engine.ast.Statement;
 import com.google.api.generator.engine.ast.ThisObjectValue;
+import com.google.api.generator.engine.ast.ThrowExpr;
+import com.google.api.generator.engine.ast.TryCatchStatement;
 import com.google.api.generator.engine.ast.TypeNode;
 import com.google.api.generator.engine.ast.ValueExpr;
 import com.google.api.generator.engine.ast.Variable;
@@ -791,6 +793,34 @@ private List<MethodDefinition> createStubOverrideMethods(
                         .build())
                 .build();
 
+    // Generate the close() method:
+    //   @Override
+    //   public final void close() {
+    //     try {
+    //       backgroundResources.close();
+    //     } catch (RuntimeException e) {
+    //       throw e;
+    //     } catch (Exception e) {
+    //       throw new IllegalStateException("Failed to close resource", e);
+    //     }
+    //  }
+
+    VariableExpr catchRuntimeExceptionVarExpr =
+        VariableExpr.builder()
+            .setVariable(
+                Variable.builder()
+                    .setType(TypeNode.withExceptionClazz(RuntimeException.class))
+                    .setName("e")
+                    .build())
+            .build();
+    VariableExpr catchExceptionVarExpr =
+        VariableExpr.builder()
+            .setVariable(
+                Variable.builder()
+                    .setType(TypeNode.withExceptionClazz(Exception.class))
+                    .setName("e")
+                    .build())
+            .build();
     List<MethodDefinition> javaMethods = new ArrayList<>();
     javaMethods.add(
         methodMakerStarterFn
@@ -799,8 +829,33 @@ private List<MethodDefinition> createStubOverrideMethods(
             .setReturnType(TypeNode.VOID)
             .setBody(
                 Arrays.asList(
-                    ExprStatement.withExpr(
-                        MethodInvocationExpr.builder().setMethodName("shutdown").build())))
+                    TryCatchStatement.builder()
+                        .setTryBody(
+                            Arrays.asList(
+                                ExprStatement.withExpr(
+                                    MethodInvocationExpr.builder()
+                                        .setExprReferenceExpr(backgroundResourcesVarExpr)
+                                        .setMethodName("close")
+                                        .build())))
+                        .addCatch(
+                            catchRuntimeExceptionVarExpr.toBuilder().setIsDecl(true).build(),
+                            Arrays.asList(
+                                ExprStatement.withExpr(
+                                    ThrowExpr.builder()
+                                        .setThrowExpr(catchRuntimeExceptionVarExpr)
+                                        .build())))
+                        .addCatch(
+                            catchExceptionVarExpr.toBuilder().setIsDecl(true).build(),
+                            Arrays.asList(
+                                ExprStatement.withExpr(
+                                    ThrowExpr.builder()
+                                        .setType(
+                                            TypeNode.withExceptionClazz(
+                                                IllegalStateException.class))
+                                        .setMessageExpr(String.format("Failed to close resource"))
+                                        .setCauseExpr(catchExceptionVarExpr)
+                                        .build())))
+                        .build()))
             .build());
     javaMethods.add(voidMethodMakerFn.apply("shutdown"));
     javaMethods.add(booleanMethodMakerFn.apply("isShutdown"));
diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcDeprecatedServiceStub.golden b/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcDeprecatedServiceStub.golden
@@ -125,7 +125,13 @@ public class GrpcDeprecatedServiceStub extends DeprecatedServiceStub {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcEchoStub.golden b/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcEchoStub.golden
@@ -341,7 +341,13 @@ public class GrpcEchoStub extends EchoStub {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcPublisherStub.golden b/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcPublisherStub.golden
@@ -430,7 +430,13 @@ public class GrpcPublisherStub extends PublisherStub {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcTestingStub.golden b/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcTestingStub.golden
@@ -382,7 +382,13 @@ public class GrpcTestingStub extends TestingStub {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/src/test/java/com/google/api/generator/gapic/composer/rest/goldens/HttpJsonComplianceStub.golden b/src/test/java/com/google/api/generator/gapic/composer/rest/goldens/HttpJsonComplianceStub.golden
@@ -484,7 +484,13 @@ public class HttpJsonComplianceStub extends ComplianceStub {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/asset/com/google/cloud/asset/v1/stub/GrpcAssetServiceStub.java b/test/integration/goldens/asset/com/google/cloud/asset/v1/stub/GrpcAssetServiceStub.java
@@ -598,7 +598,13 @@ public UnaryCallable<DeleteFeedRequest, Empty> deleteFeedCallable() {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonAddressesStub.java b/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonAddressesStub.java
@@ -403,7 +403,13 @@ public UnaryCallable<ListAddressesRequest, ListPagedResponse> listPagedCallable(
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonRegionOperationsStub.java b/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonRegionOperationsStub.java
@@ -167,7 +167,13 @@ public UnaryCallable<GetRegionOperationRequest, Operation> getCallable() {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/credentials/com/google/cloud/iam/credentials/v1/stub/GrpcIamCredentialsStub.java b/test/integration/goldens/credentials/com/google/cloud/iam/credentials/v1/stub/GrpcIamCredentialsStub.java
@@ -239,7 +239,13 @@ public UnaryCallable<SignJwtRequest, SignJwtResponse> signJwtCallable() {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/iam/com/google/iam/v1/stub/GrpcIAMPolicyStub.java b/test/integration/goldens/iam/com/google/iam/v1/stub/GrpcIAMPolicyStub.java
@@ -197,7 +197,13 @@ public UnaryCallable<GetIamPolicyRequest, Policy> getIamPolicyCallable() {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/GrpcKeyManagementServiceStub.java b/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/GrpcKeyManagementServiceStub.java
@@ -1119,7 +1119,13 @@ public UnaryCallable<GetLocationRequest, Location> getLocationCallable() {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/library/com/google/cloud/example/library/v1/stub/GrpcLibraryServiceStub.java b/test/integration/goldens/library/com/google/cloud/example/library/v1/stub/GrpcLibraryServiceStub.java
@@ -450,7 +450,13 @@ public UnaryCallable<MoveBookRequest, Book> moveBookCallable() {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/logging/com/google/cloud/logging/v2/stub/GrpcConfigServiceV2Stub.java b/test/integration/goldens/logging/com/google/cloud/logging/v2/stub/GrpcConfigServiceV2Stub.java
@@ -889,7 +889,13 @@ public UnaryCallable<UpdateCmekSettingsRequest, CmekSettings> updateCmekSettings
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/logging/com/google/cloud/logging/v2/stub/GrpcLoggingServiceV2Stub.java b/test/integration/goldens/logging/com/google/cloud/logging/v2/stub/GrpcLoggingServiceV2Stub.java
@@ -328,7 +328,13 @@ public UnaryCallable<ListLogsRequest, ListLogsPagedResponse> listLogsPagedCallab
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/logging/com/google/cloud/logging/v2/stub/GrpcMetricsServiceV2Stub.java b/test/integration/goldens/logging/com/google/cloud/logging/v2/stub/GrpcMetricsServiceV2Stub.java
@@ -279,7 +279,13 @@ public UnaryCallable<DeleteLogMetricRequest, Empty> deleteLogMetricCallable() {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/pubsub/com/google/cloud/pubsub/v1/stub/GrpcPublisherStub.java b/test/integration/goldens/pubsub/com/google/cloud/pubsub/v1/stub/GrpcPublisherStub.java
@@ -549,7 +549,13 @@ public UnaryCallable<GetIamPolicyRequest, Policy> getIamPolicyCallable() {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/pubsub/com/google/cloud/pubsub/v1/stub/GrpcSchemaServiceStub.java b/test/integration/goldens/pubsub/com/google/cloud/pubsub/v1/stub/GrpcSchemaServiceStub.java
@@ -412,7 +412,13 @@ public UnaryCallable<GetIamPolicyRequest, Policy> getIamPolicyCallable() {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/pubsub/com/google/cloud/pubsub/v1/stub/GrpcSubscriberStub.java b/test/integration/goldens/pubsub/com/google/cloud/pubsub/v1/stub/GrpcSubscriberStub.java
@@ -762,7 +762,13 @@ public UnaryCallable<GetIamPolicyRequest, Policy> getIamPolicyCallable() {
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/redis/com/google/cloud/redis/v1beta1/stub/GrpcCloudRedisStub.java b/test/integration/goldens/redis/com/google/cloud/redis/v1beta1/stub/GrpcCloudRedisStub.java
@@ -505,7 +505,13 @@ public OperationCallable<DeleteInstanceRequest, Empty, Any> deleteInstanceOperat
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
diff --git a/test/integration/goldens/storage/com/google/storage/v2/stub/GrpcStorageStub.java b/test/integration/goldens/storage/com/google/storage/v2/stub/GrpcStorageStub.java
@@ -201,7 +201,13 @@ public ClientStreamingCallable<WriteObjectRequest, WriteObjectResponse> writeObj
 
   @Override
   public final void close() {
-    shutdown();
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
   }
 
   @Override
