8359477: com/sun/net/httpserver/Test12.java appears to have a temp file race

Reviewed-by: dfuchs, vyazici

test/jdk/com/sun/net/httpserver/FileServerHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -43,7 +43,6 @@ public class FileServerHandler implements HttpHandler {
         this.docroot = docroot;
     }
 
-    int invocation = 1;
     public void handle (HttpExchange t)
         throws IOException
     {
@@ -87,16 +86,16 @@ public class FileServerHandler implements HttpHandler {
             rmap.set ("Content-Type", "text/html");
             t.sendResponseHeaders (200, 0);
             String[] list = f.list();
-            OutputStream os = t.getResponseBody();
-            PrintStream p = new PrintStream (os);
-            p.println ("<h2>Directory listing for: " + path+ "</h2>");
-            p.println ("<ul>");
-            for (int i=0; i<list.length; i++) {
-                p.println ("<li><a href=\""+list[i]+"\">"+list[i]+"</a></li>");
+            try (final OutputStream os = t.getResponseBody();
+                 final PrintStream p = new PrintStream (os)) {
+                p.println("<h2>Directory listing for: " + path + "</h2>");
+                p.println("<ul>");
+                for (int i = 0; i < list.length; i++) {
+                    p.println("<li><a href=\"" + list[i] + "\">" + list[i] + "</a></li>");
+                }
+                p.println("</ul><p><hr>");
+                p.flush();
             }
-            p.println ("</ul><p><hr>");
-            p.flush();
-            p.close();
         } else {
             int clen;
             if (fixedrequest != null) {
@@ -105,10 +104,9 @@ public class FileServerHandler implements HttpHandler {
                 clen = 0;
             }
             t.sendResponseHeaders (200, clen);
-            OutputStream os = t.getResponseBody();
-            FileInputStream fis = new FileInputStream (f);
             int count = 0;
-            try {
+            try (final OutputStream os = t.getResponseBody();
+                 final FileInputStream fis = new FileInputStream (f)) {
                 byte[] buf = new byte [16 * 1024];
                 int len;
                 while ((len=fis.read (buf)) != -1) {
@@ -118,8 +116,6 @@ public class FileServerHandler implements HttpHandler {
             } catch (IOException e) {
                 e.printStackTrace();
             }
-            fis.close();
-            os.close();
         }
     }
 

test/jdk/com/sun/net/httpserver/Test12.java

@@ -21,23 +21,12 @@
  * questions.
  */
 
-/*
- * @test
- * @bug 6270015
- * @library /test/lib
- * @build jdk.test.lib.Asserts
- *        jdk.test.lib.Utils
- *        jdk.test.lib.net.SimpleSSLContext
- *        jdk.test.lib.net.URIBuilder
- * @run main/othervm Test12
- * @run main/othervm -Djava.net.preferIPv6Addresses=true Test12
- * @summary Light weight HTTP server
- */
-
 import com.sun.net.httpserver.*;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.concurrent.*;
 import java.io.*;
 import java.net.*;
@@ -49,11 +38,19 @@ import static jdk.test.lib.Asserts.assertEquals;
 import static jdk.test.lib.Asserts.assertFileContentsEqual;
 import static jdk.test.lib.Utils.createTempFileOfSize;
 
-/* basic http/s connectivity test
- * Tests:
- *      - same as Test1, but in parallel
+/*
+ * @test
+ * @bug 6270015 8359477
+ * @summary Light weight HTTP server - basic http/s connectivity test, same as Test1,
+ *          but in parallel
+ * @library /test/lib
+ * @build jdk.test.lib.Asserts
+ *        jdk.test.lib.Utils
+ *        jdk.test.lib.net.SimpleSSLContext
+ *        jdk.test.lib.net.URIBuilder
+ * @run main/othervm Test12
+ * @run main/othervm -Djava.net.preferIPv6Addresses=true Test12
  */
-
 public class Test12 extends Test {
 
     private static final String TEMP_FILE_PREFIX =
@@ -61,14 +58,12 @@ public class Test12 extends Test {
 
     static SSLContext ctx;
 
-    static boolean fail = false;
-
     public static void main (String[] args) throws Exception {
         HttpServer s1 = null;
         HttpsServer s2 = null;
-        ExecutorService executor=null;
         Path smallFilePath = createTempFileOfSize(TEMP_FILE_PREFIX, null, 23);
         Path largeFilePath = createTempFileOfSize(TEMP_FILE_PREFIX, null, 2730088);
+        final ExecutorService executor = Executors.newCachedThreadPool();
         try {
             System.out.print ("Test12: ");
             InetAddress loopback = InetAddress.getLoopbackAddress();
@@ -80,7 +75,6 @@ public class Test12 extends Test {
             HttpHandler h = new FileServerHandler(smallFilePath.getParent().toString());
             HttpContext c1 = s1.createContext ("/", h);
             HttpContext c2 = s2.createContext ("/", h);
-            executor = Executors.newCachedThreadPool();
             s1.setExecutor (executor);
             s2.setExecutor (executor);
             ctx = new SimpleSSLContext().get();
@@ -90,7 +84,7 @@ public class Test12 extends Test {
 
             int port = s1.getAddress().getPort();
             int httpsport = s2.getAddress().getPort();
-            Runner r[] = new Runner[8];
+            final Runner[] r = new Runner[8];
             r[0] = new Runner (true, "http", port, smallFilePath);
             r[1] = new Runner (true, "http", port, largeFilePath);
             r[2] = new Runner (true, "https", httpsport, smallFilePath);
@@ -99,95 +93,83 @@ public class Test12 extends Test {
             r[5] = new Runner (false, "http", port, largeFilePath);
             r[6] = new Runner (false, "https", httpsport, smallFilePath);
             r[7] = new Runner (false, "https", httpsport, largeFilePath);
-            start (r);
-            join (r);
-            System.out.println ("OK");
+            // submit the tasks
+            final List<Future<Void>> futures = new ArrayList<>();
+            for (Runner runner : r) {
+                futures.add(executor.submit(runner));
+            }
+            // wait for the tasks' completion
+            for (Future<Void> f : futures) {
+                f.get();
+            }
+            System.out.println ("All " + futures.size() + " tasks completed successfully");
         } finally {
-            if (s1 != null)
+            if (s1 != null) {
                 s1.stop(0);
-            if (s2 != null)
+            }
+            if (s2 != null) {
                 s2.stop(0);
-            if (executor != null)
-                executor.shutdown ();
+            }
+            executor.close();
+            // it's OK to delete these files since the server side handlers
+            // serving these files have completed (guaranteed by the completion of Executor.close())
+            System.out.println("deleting " + smallFilePath);
             Files.delete(smallFilePath);
+            System.out.println("deleting " + largeFilePath);
             Files.delete(largeFilePath);
         }
     }
 
-    static void start (Runner[] x) {
-        for (int i=0; i<x.length; i++) {
-            x[i].start();
-        }
-    }
-
-    static void join (Runner[] x) {
-        for (int i=0; i<x.length; i++) {
-            try {
-                x[i].join();
-            } catch (InterruptedException e) {}
-        }
-    }
-
-
-    static class Runner extends Thread {
+    static class Runner implements Callable<Void> {
 
         boolean fixedLen;
         String protocol;
         int port;
         private final Path filePath;
 
-        Runner (boolean fixedLen, String protocol, int port, Path filePath) {
+        Runner(boolean fixedLen, String protocol, int port, Path filePath) {
             this.fixedLen=fixedLen;
             this.protocol=protocol;
             this.port=port;
             this.filePath = filePath;
         }
 
-        public void run () {
-            try {
-                URL url = URIBuilder.newBuilder()
-                          .scheme(protocol)
-                          .loopback()
-                          .port(port)
-                          .path("/" + filePath.getFileName())
-                          .toURL();
-                HttpURLConnection urlc = (HttpURLConnection) url.openConnection(Proxy.NO_PROXY);
-                if (urlc instanceof HttpsURLConnection) {
-                    HttpsURLConnection urlcs = (HttpsURLConnection) urlc;
-                    urlcs.setHostnameVerifier (new HostnameVerifier () {
-                        public boolean verify (String s, SSLSession s1) {
-                            return true;
-                        }
-                    });
-                    urlcs.setSSLSocketFactory (ctx.getSocketFactory());
-                }
-                byte [] buf = new byte [4096];
-
-                if (fixedLen) {
-                    urlc.setRequestProperty ("XFixed", "yes");
-                }
-                InputStream is = urlc.getInputStream();
-                File temp = File.createTempFile ("Test1", null);
-                temp.deleteOnExit();
-                OutputStream fout = new BufferedOutputStream (new FileOutputStream(temp));
-                int c, count = 0;
-                while ((c=is.read(buf)) != -1) {
-                    count += c;
-                    fout.write (buf, 0, c);
-                }
-                is.close();
-                fout.close();
-
-                if (count != filePath.toFile().length()) {
-                    throw new RuntimeException ("wrong amount of data returned");
-                }
-                assertFileContentsEqual(filePath, temp.toPath());
-                temp.delete();
-            } catch (Exception e) {
-                e.printStackTrace();
-                fail = true;
+        @Override
+        public Void call() throws Exception {
+            final URL url = URIBuilder.newBuilder()
+                      .scheme(protocol)
+                      .loopback()
+                      .port(port)
+                      .path("/" + filePath.getFileName())
+                      .toURL();
+            final HttpURLConnection urlc = (HttpURLConnection) url.openConnection(Proxy.NO_PROXY);
+            if (urlc instanceof HttpsURLConnection) {
+                HttpsURLConnection urlcs = (HttpsURLConnection) urlc;
+                urlcs.setHostnameVerifier (new HostnameVerifier () {
+                    public boolean verify (String s, SSLSession s1) {
+                        return true;
+                    }
+                });
+                urlcs.setSSLSocketFactory (ctx.getSocketFactory());
             }
+            if (fixedLen) {
+                urlc.setRequestProperty ("XFixed", "yes");
+            }
+            final Path temp = Files.createTempFile(Path.of("."), "Test12", null);
+            final long numReceived;
+            try (InputStream is = urlc.getInputStream();
+                 OutputStream fout = new BufferedOutputStream(new FileOutputStream(temp.toFile()))) {
+                numReceived = is.transferTo(fout);
+            }
+            System.out.println("received " + numReceived + " response bytes for " + url);
+            final long expected = filePath.toFile().length();
+            if (numReceived != expected) {
+                throw new RuntimeException ("expected " + expected + " bytes, but received "
+                        + numReceived);
+            }
+            assertFileContentsEqual(filePath, temp);
+            Files.delete(temp);
+            return null;
         }
     }
-
 }