Don't convert InterruptException to EvalException in struct field call expressions.

This is probably only a theoretical problem, since a blocking struct field is probably a very bad idea.

Closes #5132.

Change-Id: Ie84a78ab4d9ce215f2806ac49bf8911de6959930
PiperOrigin-RevId: 207902766

3 files changed, 40 insertions, 5 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
index 469df1e3e7..70d2bbff30 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
@@ -179,10 +179,8 @@ public class BuiltinFunction extends BaseFunction {
       } else if (e instanceof IllegalArgumentException) {
         throw new EvalException(loc, "illegal argument in call to " + getName(), e);
       }
-      // TODO(bazel-team): replace with Throwables.throwIfInstanceOf once Guava 20 is released.
-      Throwables.propagateIfInstanceOf(e, InterruptedException.class);
-      // TODO(bazel-team): replace with Throwables.throwIfUnchecked once Guava 20 is released.
-      Throwables.propagateIfPossible(e);
+      Throwables.throwIfInstanceOf(e, InterruptedException.class);
+      Throwables.throwIfUnchecked(e);
       throw badCallException(loc, e, args);
     } catch (IllegalArgumentException e) {
       // Either this was thrown by Java itself, or it's a bug
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 44e6a67ae9..03695abaa2 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.syntax;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Throwables;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
@@ -781,20 +782,23 @@ public final class FuncallExpression extends Expression {
           call.findJavaMethod(objClass, method, positionalArgs, keyWordArgs, env);
       if (javaMethod.first.isStructField()) {
         // Not a method but a callable attribute
+        Object func;
         try {
-          return callFunction(javaMethod.first.invoke(obj), env);
+          func = javaMethod.first.invoke(obj);
         } catch (IllegalAccessException e) {
           throw new EvalException(getLocation(), "method invocation failed: " + e);
         } catch (InvocationTargetException e) {
           if (e.getCause() instanceof FuncallException) {
             throw new EvalException(getLocation(), e.getCause().getMessage());
           } else if (e.getCause() != null) {
+            Throwables.throwIfInstanceOf(e.getCause(), InterruptedException.class);
             throw new EvalExceptionWithJavaCause(getLocation(), e.getCause());
           } else {
             // This is unlikely to happen
             throw new EvalException(getLocation(), "method invocation failed: " + e);
           }
         }
+        return callFunction(func, env);
       }
       return javaMethod.first.call(obj, javaMethod.second.toArray(), location, env);
     }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index ecbe65cec5..6991363171 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.syntax;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 import static java.util.stream.Collectors.joining;
 
 import com.google.common.collect.ImmutableCollection;
@@ -73,6 +74,17 @@ public class SkylarkEvaluationTest extends EvaluationTest {
     }
   };
 
+  @SkylarkSignature(
+      name = "interrupted_function",
+      returnType = Runtime.NoneType.class,
+      documented = false)
+  static BuiltinFunction interruptedFunction =
+      new BuiltinFunction("interrupted_function") {
+        public Runtime.NoneType invoke() throws InterruptedException {
+          throw new InterruptedException();
+        }
+      };
+
   @SkylarkModule(name = "Mock", doc = "")
   static class NativeInfoMock extends NativeInfo {
 
@@ -154,6 +166,12 @@ public class SkylarkEvaluationTest extends EvaluationTest {
     public BuiltinFunction structFieldCallable() {
       return foobar;
     }
+
+    @SkylarkCallable(name = "interrupted_struct_field", documented = false, structField = true)
+    public BuiltinFunction structFieldInterruptedCallable() throws InterruptedException {
+      throw new InterruptedException();
+    }
+
     @SkylarkCallable(name = "function", documented = false, structField = false)
     public String function() {
       return "a";
@@ -1261,6 +1279,20 @@ public class SkylarkEvaluationTest extends EvaluationTest {
   }
 
   @Test
+  public void testCallingInterruptedStructField() throws Exception {
+    update("mock", new Mock());
+    assertThrows(InterruptedException.class, () -> eval("mock.interrupted_struct_field()"));
+  }
+
+  @Test
+  public void testCallingInterruptedFunction() throws Exception {
+    interruptedFunction.configure(
+        getClass().getDeclaredField("interruptedFunction").getAnnotation(SkylarkSignature.class));
+    update("interrupted_function", interruptedFunction);
+    assertThrows(InterruptedException.class, () -> eval("interrupted_function()"));
+  }
+
+  @Test
   public void testJavaFunctionWithExtraInterpreterParams() throws Exception {
     new SkylarkTest()
         .update("mock", new Mock())
@@ -1868,6 +1900,7 @@ public class SkylarkEvaluationTest extends EvaluationTest {
         .testExactOrder(
             "dir(mock)",
             "function",
+            "interrupted_struct_field",
             "is_empty",
             "legacy_method",
             "nullfunc_failing",