Import of Cloud Functions JVM from Git-on-Borg.

  - 0b5e0e335b3a58b848eebae4c025926e0aa9d33f Fix warnings about reflection on platform classes. by Éamonn McManus <[email protected]>

PiperOrigin-RevId: 288948744

Author: eamonnmcmanus

invoker-core/core/src/main/java/com/google/cloud/functions/invoker/http/HttpResponseImpl.java

@@ -6,9 +6,6 @@
 import java.io.BufferedWriter;
 import java.io.IOException;
 import java.io.OutputStream;
-import java.io.PrintWriter;
-import java.io.Writer;
-import java.lang.reflect.Field;
 import java.util.AbstractMap.SimpleEntry;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -70,26 +67,14 @@ public OutputStream getOutputStream() throws IOException {
 
   @Override
   public synchronized BufferedWriter getWriter() throws IOException {
-    if (writer != null) {
-      return writer;
+    if (writer == null) {
+      // Unfortunately this means that we get two intermediate objects between the object we return
+      // and the underlying Writer that response.getWriter() wraps. We could try accessing the
+      // PrintWriter.out field via reflection, but that sort of access to non-public fields of
+      // platform classes is now frowned on and may draw warnings or even fail in subsequent
+      // versions.
+      writer = new BufferedWriter(response.getWriter());
     }
-    // We could just wrap a BufferedWriter around the PrintWriter that the Servlet API gives us,
-    // but this slightly clunky alternative potentially avoids two intermediate objects in the
-    // writer chain.
-    PrintWriter printWriter = response.getWriter();
-    Writer wrappedWriter;
-    try {
-      // This is a protected field, so it is part of the documented API and we know it will be
-      // there, but we need to use reflection to get at it.
-      Field outField = PrintWriter.class.getDeclaredField("out");
-      outField.setAccessible(true);
-      wrappedWriter = (Writer) outField.get(printWriter);
-    } catch (ReflectiveOperationException e) {
-      throw new IOException("Reflection failed", e);
-    }
-    this.writer = (wrappedWriter instanceof BufferedWriter)
-        ? (BufferedWriter) wrappedWriter
-        : new BufferedWriter(wrappedWriter);
-    return this.writer;
+    return writer;
   }
 }

invoker-core/core/src/test/java/com/google/cloud/functions/invoker/IntegrationTest.java

@@ -28,6 +28,9 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.regex.Pattern;
 import org.eclipse.jetty.client.HttpClient;
@@ -47,6 +50,8 @@ public class IntegrationTest {
 
   private static final String SERVER_READY_STRING = "Started ServerConnector";
 
+  private static final ExecutorService EXECUTOR = Executors.newCachedThreadPool();
+
   private static int serverPort;
 
   /**
@@ -264,7 +269,7 @@ private void testFunction(
       String target,
       ImmutableList<String> extraArgs,
       TestCase... testCases) throws Exception {
-    Process server = startServer(signatureType, target, extraArgs);
+    ServerProcess serverProcess = startServer(signatureType, target, extraArgs);
     try {
       HttpClient httpClient = new HttpClient();
       httpClient.start();
@@ -280,9 +285,11 @@ private void testFunction(
         assertThat(response.getContentAsString()).isEqualTo(testCase.expectedResponseText());
       }
     } finally {
-      server.destroy();
-      server.waitFor();
+      serverProcess.close();
     }
+    // Wait for the output monitor task to terminate. If it threw an exception, we will get an
+    // ExecutionException here.
+    serverProcess.outputMonitorResult().get();
   }
 
   private enum SignatureType {
@@ -301,7 +308,22 @@ public String toString() {
     }
   }
 
-  private Process startServer(
+  @AutoValue
+  abstract static class ServerProcess implements AutoCloseable {
+    abstract Process process();
+    abstract Future<?> outputMonitorResult();
+
+    static ServerProcess of(Process process, Future<?> outputMonitorResult) {
+      return new AutoValue_IntegrationTest_ServerProcess(process, outputMonitorResult);
+    }
+
+    @Override
+    public void close() {
+      process().destroy();
+    }
+  }
+
+  private ServerProcess startServer(
       SignatureType signatureType, String target, ImmutableList<String> extraArgs)
       throws IOException, InterruptedException {
     File javaHome = new File(System.getProperty("java.home"));
@@ -325,10 +347,11 @@ private Process startServer(
     processBuilder.environment().putAll(environment);
     Process serverProcess = processBuilder.start();
     CountDownLatch ready = new CountDownLatch(1);
-    new Thread(() -> monitorOutput(serverProcess.getInputStream(), ready)).start();
+    Future<?> outputMonitorResult = EXECUTOR.submit(
+        () -> monitorOutput(serverProcess.getInputStream(), ready));
     boolean serverReady = ready.await(5, TimeUnit.SECONDS);
     assertWithMessage("Waiting for server to be ready").that(serverReady).isTrue();
-    return serverProcess;
+    return ServerProcess.of(serverProcess, outputMonitorResult);
   }
 
   private void monitorOutput(InputStream processOutput, CountDownLatch ready) {
@@ -339,6 +362,9 @@ private void monitorOutput(InputStream processOutput, CountDownLatch ready) {
           ready.countDown();
         }
         System.out.println(line);
+        if (line.contains("WARNING")) {
+          throw new AssertionError("Found warning in server output:\n" + line);
+        }
       }
     } catch (IOException e) {
       e.printStackTrace();