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

src/main/java/com/google/api/generator/engine/ast/ThrowExpr.java

@@ -22,6 +22,9 @@
 public abstract class ThrowExpr implements Expr {
   // TODO(miraleung): Refactor with StringObjectValue and possibly with NewObjectExpr.
 
+  @Nullable
+  public abstract Expr throwExpr();
+
   @Override
   public abstract TypeNode type();
 
@@ -42,6 +45,9 @@ public static Builder builder() {
 
   @AutoValue.Builder
   public abstract static class Builder {
+    public abstract Builder setThrowExpr(Expr throwExpr);
+
+    // No-op if setThrowExpr is called.
     public abstract Builder setType(TypeNode type);
 
     public Builder setMessageExpr(String message) {
@@ -53,6 +59,8 @@ public Builder setMessageExpr(String message) {
     public abstract Builder setCauseExpr(Expr expr);
 
     // Private.
+    abstract Expr throwExpr();
+
     abstract TypeNode type();
 
     abstract Expr messageExpr();
@@ -62,6 +70,24 @@ public Builder setMessageExpr(String message) {
     abstract ThrowExpr autoBuild();
 
     public ThrowExpr build() {
+      if (throwExpr() != null) {
+        setType(throwExpr().type());
+        Preconditions.checkState(
+            messageExpr() == null && causeExpr() == null,
+            "Only one of throwExpr or [messageExpr or causeExpr, inclusive] can be present.");
+
+        if (throwExpr() instanceof VariableExpr) {
+          Preconditions.checkState(
+              !((VariableExpr) throwExpr()).isDecl(), "Cannot throw a variable declaration");
+        }
+
+        Preconditions.checkState(
+            TypeNode.isExceptionType(throwExpr().type()),
+            String.format("Only exception types can be thrown, found %s", throwExpr().type()));
+
+        return autoBuild();
+      }
+
       Preconditions.checkState(
           TypeNode.isExceptionType(type()),
           String.format("Type %s must be an exception type", type()));

src/main/java/com/google/api/generator/engine/ast/TryCatchStatement.java

@@ -17,20 +17,23 @@
 import com.google.auto.value.AutoValue;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 @AutoValue
 public abstract class TryCatchStatement implements Statement {
 
   // Required.
   public abstract ImmutableList<Statement> tryBody();
+
   // Optional only if the sample code bit is set (i.e. this is sample code).
-  @Nullable
-  public abstract VariableExpr catchVariableExpr();
+  public abstract List<VariableExpr> catchVariableExprs();
   // Optional only if the sample code bit is set (i.e. this is sample code).
-  public abstract ImmutableList<Statement> catchBody();
+  public abstract List<List<Statement>> catchBlocks();
+
   // Optional.
   @Nullable
   public abstract AssignmentExpr tryResourceExpr();
@@ -44,8 +47,9 @@ public void accept(AstNodeVisitor visitor) {
 
   public static Builder builder() {
     return new AutoValue_TryCatchStatement.Builder()
-        .setIsSampleCode(false)
-        .setCatchBody(Collections.emptyList());
+        .setCatchVariableExprs(Collections.emptyList())
+        .setCatchBlocks(Collections.emptyList())
+        .setIsSampleCode(false);
   }
 
   @AutoValue.Builder
@@ -54,32 +58,61 @@ public abstract static class Builder {
 
     public abstract Builder setTryBody(List<Statement> body);
 
-    public abstract Builder setCatchVariableExpr(VariableExpr variableExpr);
+    public abstract Builder setIsSampleCode(boolean isSampleCode);
 
-    public abstract Builder setCatchBody(List<Statement> body);
+    public Builder addCatch(@Nonnull VariableExpr variableExpr, List<Statement> body) {
+      List<VariableExpr> catchVarExprs = new ArrayList<>(catchVariableExprs());
+      catchVarExprs.add(variableExpr);
+      setCatchVariableExprs(catchVarExprs);
 
-    public abstract Builder setIsSampleCode(boolean isSampleCode);
+      List<List<Statement>> blocks = new ArrayList<>(catchBlocks());
+      blocks.add(body);
+      return setCatchBlocks(blocks);
+    }
+
+    // Private.
+    abstract Builder setCatchVariableExprs(List<VariableExpr> variableExpr);
+
+    abstract Builder setCatchBlocks(List<List<Statement>> body);
+
+    abstract ImmutableList<Statement> tryBody();
+
+    abstract boolean isSampleCode();
+
+    abstract List<VariableExpr> catchVariableExprs();
+
+    abstract List<List<Statement>> catchBlocks();
 
     abstract TryCatchStatement autoBuild();
 
     public TryCatchStatement build() {
-      TryCatchStatement tryCatchStatement = autoBuild();
-      NodeValidator.checkNoNullElements(tryCatchStatement.tryBody(), "try body", "try-catch");
-      NodeValidator.checkNoNullElements(tryCatchStatement.catchBody(), "catch body", "try-catch");
+      NodeValidator.checkNoNullElements(tryBody(), "try body", "try-catch");
+      NodeValidator.checkNoNullElements(
+          catchVariableExprs(), "catch variable expressions", "try-catch");
+      catchBlocks()
+          .forEach(body -> NodeValidator.checkNoNullElements(body, "catch body", "try-catch"));
 
-      if (!tryCatchStatement.isSampleCode()) {
-        Preconditions.checkNotNull(
-            tryCatchStatement.catchVariableExpr(),
+      if (!isSampleCode()) {
+        Preconditions.checkState(
+            !catchVariableExprs().isEmpty(),
             "Catch variable expression must be set for real, non-sample try-catch blocks.");
         Preconditions.checkState(
-            tryCatchStatement.catchVariableExpr().isDecl(),
-            "Catch variable expression must be a declaration");
+            catchVariableExprs().stream().allMatch(v -> v.isDecl()),
+            "Catch variable expressions must all be declarations");
         Preconditions.checkState(
-            TypeNode.isExceptionType(tryCatchStatement.catchVariableExpr().variable().type()),
-            "Catch variable must be an Exception object reference");
+            catchVariableExprs().stream()
+                .allMatch(v -> TypeNode.isExceptionType(v.variable().type())),
+            "Catch variables must be an Exception object references");
       }
 
-      return tryCatchStatement;
+      // Catch any potential future breakage due to changing addCatch above.
+      Preconditions.checkState(
+          catchVariableExprs().size() == catchBlocks().size(),
+          String.format(
+              "%d catch variables found and %d blocks found, but these numbers must be equal",
+              catchVariableExprs().size(), catchBlocks().size()));
+
+      return autoBuild();
     }
   }
 }

src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java

@@ -231,6 +231,13 @@ public void visit(AnonymousClassExpr anonymousClassExpr) {
   @Override
   public void visit(ThrowExpr throwExpr) {
     throwExpr.type().accept(this);
+    // If throwExpr is present, then messageExpr and causeExpr will not be present. Relies on AST
+    // build-time checks.
+    if (throwExpr.throwExpr() != null) {
+      throwExpr.throwExpr().accept(this);
+      return;
+    }
+
     if (throwExpr.messageExpr() != null) {
       throwExpr.messageExpr().accept(this);
     }
@@ -355,11 +362,13 @@ public void visit(TryCatchStatement tryCatchStatement) {
     statements(tryCatchStatement.tryBody());
 
     Preconditions.checkState(
-        !tryCatchStatement.isSampleCode() && tryCatchStatement.catchVariableExpr() != null,
+        !tryCatchStatement.isSampleCode() && !tryCatchStatement.catchVariableExprs().isEmpty(),
         "Import generation should not be invoked on sample code, but was found when visiting a"
             + " try-catch block");
-    tryCatchStatement.catchVariableExpr().accept(this);
-    statements(tryCatchStatement.catchBody());
+    for (int i = 0; i < tryCatchStatement.catchVariableExprs().size(); i++) {
+      tryCatchStatement.catchVariableExprs().get(i).accept(this);
+      statements(tryCatchStatement.catchBlocks().get(i));
+    }
   }
 
   @Override

src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java

@@ -392,6 +392,13 @@ public void visit(AnonymousClassExpr anonymousClassExpr) {
   public void visit(ThrowExpr throwExpr) {
     buffer.append(THROW);
     space();
+    // If throwExpr is present, then messageExpr and causeExpr will not be present. Relies on AST
+    // build-time checks.
+    if (throwExpr.throwExpr() != null) {
+      throwExpr.throwExpr().accept(this);
+      return;
+    }
+
     buffer.append(NEW);
     space();
     throwExpr.type().accept(this);
@@ -696,17 +703,17 @@ public void visit(TryCatchStatement tryCatchStatement) {
     statements(tryCatchStatement.tryBody());
     rightBrace();
 
-    if (tryCatchStatement.catchVariableExpr() != null) {
+    for (int i = 0; i < tryCatchStatement.catchVariableExprs().size(); i++) {
       space();
       buffer.append(CATCH);
       space();
       leftParen();
-      tryCatchStatement.catchVariableExpr().accept(this);
+      tryCatchStatement.catchVariableExprs().get(i).accept(this);
       rightParen();
       space();
       leftBrace();
       newline();
-      statements(tryCatchStatement.catchBody());
+      statements(tryCatchStatement.catchBlocks().get(i));
       rightBrace();
     }
     newline();

src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceClientTestClassComposer.java

@@ -536,7 +536,8 @@ private MethodDefinition createRpcTestMethod(
                       TypeNode.withReference(
                           ConcreteReference.builder()
                               .setClazz(List.class)
-                              .setGenerics(Arrays.asList(repeatedPagedResultsField.type().reference()))
+                              .setGenerics(
+                                  Arrays.asList(repeatedPagedResultsField.type().reference()))
                               .build()))
                   .setName("resources")
                   .build());
@@ -824,8 +825,7 @@ protected List<Statement> createRpcExceptionTestStatements(
                 tryBodyExprs.stream()
                     .map(e -> ExprStatement.withExpr(e))
                     .collect(Collectors.toList()))
-            .setCatchVariableExpr(catchExceptionVarExpr.toBuilder().setIsDecl(true).build())
-            .setCatchBody(catchBody)
+            .addCatch(catchExceptionVarExpr.toBuilder().setIsDecl(true).build(), catchBody)
             .build();
 
     return Arrays.asList(EMPTY_LINE_STATEMENT, tryCatchBlock);

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"));

src/main/java/com/google/api/generator/gapic/composer/grpc/ServiceClientTestClassComposer.java

@@ -1046,8 +1046,9 @@ protected List<Statement> createStreamingRpcExceptionTestStatements(
                 tryBodyExprs.stream()
                     .map(e -> ExprStatement.withExpr(e))
                     .collect(Collectors.toList()))
-            .setCatchVariableExpr(catchExceptionVarExpr.toBuilder().setIsDecl(true).build())
-            .setCatchBody(createRpcLroExceptionTestCatchBody(catchExceptionVarExpr, true))
+            .addCatch(
+                catchExceptionVarExpr.toBuilder().setIsDecl(true).build(),
+                createRpcLroExceptionTestCatchBody(catchExceptionVarExpr, true))
             .build();
 
     statements.add(tryCatchBlock);

src/test/java/com/google/api/generator/engine/JavaCodeGeneratorTest.java

@@ -606,8 +606,8 @@ private MethodDefinition createPrintShelfListToFile() {
                     loopShelfList,
                     ExprStatement.withExpr(writeToFileWriter),
                     ExprStatement.withExpr(closeFileWriter)))
-            .setCatchVariableExpr(createVarDeclExpr(ioException))
-            .setCatchBody(Arrays.asList(ExprStatement.withExpr(printError)))
+            .addCatch(
+                createVarDeclExpr(ioException), Arrays.asList(ExprStatement.withExpr(printError)))
             .build();
 
     return MethodDefinition.builder()