changeset 57639:6d386d360955

8234466: Class loading deadlock involving X509Factory#commitEvent() Reviewed-by: alanb, chegar, dfuchs
author coffeys
date Mon, 13 Jan 2020 21:16:27 +0000
parents 9338d0f52b2e
children db9bdbeaed29
files src/java.base/share/classes/java/util/jar/JarFile.java src/java.base/share/classes/java/util/jar/JavaUtilJarAccessImpl.java src/java.base/share/classes/jdk/internal/access/JavaUtilJarAccess.java src/java.base/share/classes/jdk/internal/event/EventHelper.java test/jdk/java/util/jar/JarFile/jarVerification/FooService.java test/jdk/java/util/jar/JarFile/jarVerification/MultiProviderTest.java test/jdk/java/util/jar/JarFile/jarVerification/MultiThreadLoad.java test/jdk/java/util/jar/JarFile/jarVerification/logging.properties
diffstat 8 files changed, 405 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/util/jar/JarFile.java	Mon Jan 13 11:51:45 2020 -0500
+++ b/src/java.base/share/classes/java/util/jar/JarFile.java	Mon Jan 13 21:16:27 2020 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2020, 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
@@ -157,8 +157,10 @@
     private boolean jvInitialized;
     private boolean verify;
     private final Runtime.Version version;  // current version
-    private final int versionFeature;         // version.feature()
+    private final int versionFeature;       // version.feature()
     private boolean isMultiRelease;         // is jar multi-release?
+    static final ThreadLocal<Boolean> isInitializing =
+            ThreadLocal.withInitial(() -> Boolean.FALSE);
 
     // indicates if Class-Path attribute present
     private boolean hasClassPathAttribute;
@@ -1031,8 +1033,13 @@
             throw new RuntimeException(e);
         }
         if (jv != null && !jvInitialized) {
-            initializeVerifier();
-            jvInitialized = true;
+            isInitializing.set(Boolean.TRUE);
+            try {
+                initializeVerifier();
+                jvInitialized = true;
+            } finally {
+                isInitializing.set(Boolean.FALSE);
+            }
         }
     }
 
--- a/src/java.base/share/classes/java/util/jar/JavaUtilJarAccessImpl.java	Mon Jan 13 11:51:45 2020 -0500
+++ b/src/java.base/share/classes/java/util/jar/JavaUtilJarAccessImpl.java	Mon Jan 13 21:16:27 2020 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2020, 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
@@ -68,4 +68,8 @@
     public void ensureInitialization(JarFile jar) {
         jar.ensureInitialization();
     }
+
+    public Boolean isInitializing() {
+        return JarFile.isInitializing.get();
+    }
 }
--- a/src/java.base/share/classes/jdk/internal/access/JavaUtilJarAccess.java	Mon Jan 13 11:51:45 2020 -0500
+++ b/src/java.base/share/classes/jdk/internal/access/JavaUtilJarAccess.java	Mon Jan 13 21:16:27 2020 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2020, 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
@@ -45,4 +45,5 @@
     public List<Object> getManifestDigests(JarFile jar);
     public Attributes getTrustedAttributes(Manifest man, String name);
     public void ensureInitialization(JarFile jar);
+    public Boolean isInitializing();
 }
--- a/src/java.base/share/classes/jdk/internal/event/EventHelper.java	Mon Jan 13 11:51:45 2020 -0500
+++ b/src/java.base/share/classes/jdk/internal/event/EventHelper.java	Mon Jan 13 21:16:27 2020 +0000
@@ -25,6 +25,11 @@
 
 package jdk.internal.event;
 
+import jdk.internal.access.JavaUtilJarAccess;
+import jdk.internal.access.SharedSecrets;
+
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Date;
@@ -37,14 +42,24 @@
 
 public final class EventHelper {
 
+    private static final JavaUtilJarAccess JUJA = SharedSecrets.javaUtilJarAccess();
+    private static volatile boolean loggingSecurity;
+    private static volatile System.Logger securityLogger;
+    private static final VarHandle LOGGER_HANDLE;
+    static {
+        try {
+            LOGGER_HANDLE =
+                    MethodHandles.lookup().findStaticVarHandle(
+                            EventHelper.class, "securityLogger", System.Logger.class);
+        } catch (ReflectiveOperationException e) {
+            throw new Error(e);
+        }
+    }
     private static final System.Logger.Level LOG_LEVEL = System.Logger.Level.DEBUG;
 
     // helper class used for logging security related events for now
     private static final String SECURITY_LOGGER_NAME = "jdk.event.security";
-    private static final System.Logger SECURITY_LOGGER =
-            System.getLogger(SECURITY_LOGGER_NAME);
-    private static final boolean LOGGING_SECURITY =
-            SECURITY_LOGGER.isLoggable(LOG_LEVEL);
+
 
     public static void logTLSHandshakeEvent(Instant start,
                                             String peerHost,
@@ -52,8 +67,9 @@
                                             String cipherSuite,
                                             String protocolVersion,
                                             long peerCertId) {
+        assert securityLogger != null;
         String prepend = getDurationString(start);
-        SECURITY_LOGGER.log(LOG_LEVEL, prepend +
+        securityLogger.log(LOG_LEVEL, prepend +
         " TLSHandshake: {0}:{1,number,#}, {2}, {3}, {4,number,#}",
         peerHost, peerPort, protocolVersion, cipherSuite, peerCertId);
     }
@@ -61,18 +77,18 @@
     public static void logSecurityPropertyEvent(String key,
                                                 String value) {
 
-        if (isLoggingSecurity()) {
-            SECURITY_LOGGER.log(LOG_LEVEL,
-                "SecurityPropertyModification: key:{0}, value:{1}", key, value);
-        }
+        assert securityLogger != null;
+        securityLogger.log(LOG_LEVEL,
+            "SecurityPropertyModification: key:{0}, value:{1}", key, value);
     }
 
     public static void logX509ValidationEvent(int anchorCertId,
                                          int[] certIds) {
+        assert securityLogger != null;
         String codes = IntStream.of(certIds)
                 .mapToObj(Integer::toString)
                 .collect(Collectors.joining(", "));
-        SECURITY_LOGGER.log(LOG_LEVEL,
+        securityLogger.log(LOG_LEVEL,
                 "ValidationChain: {0,number,#}, {1}", anchorCertId, codes);
     }
 
@@ -85,7 +101,8 @@
                                                long certId,
                                                long beginDate,
                                                long endDate) {
-        SECURITY_LOGGER.log(LOG_LEVEL, "X509Certificate: Alg:{0}, Serial:{1}" +
+        assert securityLogger != null;
+        securityLogger.log(LOG_LEVEL, "X509Certificate: Alg:{0}, Serial:{1}" +
             ", Subject:{2}, Issuer:{3}, Key type:{4}, Length:{5,number,#}" +
             ", Cert Id:{6,number,#}, Valid from:{7}, Valid until:{8}",
             algId, serialNum, subject, issuer, keyType, length,
@@ -124,6 +141,14 @@
      * @return boolean indicating whether an event should be logged
      */
     public static boolean isLoggingSecurity() {
-        return LOGGING_SECURITY;
+        // Avoid a bootstrap issue where the commitEvent attempts to
+        // trigger early loading of System Logger but where
+        // the verification process still has JarFiles locked
+        if (securityLogger == null && !JUJA.isInitializing()) {
+            LOGGER_HANDLE.compareAndSet( null, System.getLogger(SECURITY_LOGGER_NAME));
+            loggingSecurity = securityLogger.isLoggable(LOG_LEVEL);
+        }
+        return loggingSecurity;
     }
+
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/jar/JarFile/jarVerification/FooService.java	Mon Jan 13 21:16:27 2020 +0000
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2020, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+public abstract class FooService { }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/jar/JarFile/jarVerification/MultiProviderTest.java	Mon Jan 13 21:16:27 2020 +0000
@@ -0,0 +1,185 @@
+/*
+ * Copyright (c) 2020, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8234466
+ * @summary attempt to trigger class loading from the classloader
+ * during JAR file verification
+ * @library /test/lib
+ * @build jdk.test.lib.compiler.CompilerUtils
+ *        jdk.test.lib.process.*
+ *        jdk.test.lib.util.JarUtils
+ *        jdk.test.lib.JDKToolLauncher
+ *        MultiThreadLoad FooService
+ * @modules java.base/jdk.internal.access:+open
+ * @run main MultiProviderTest
+ * @run main MultiProviderTest sign
+ */
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+
+import jdk.test.lib.JDKToolFinder;
+import jdk.test.lib.JDKToolLauncher;
+import jdk.test.lib.Utils;
+import jdk.test.lib.compiler.CompilerUtils;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.util.JarUtils;
+
+import static java.nio.file.StandardOpenOption.CREATE;
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
+import static java.util.Arrays.asList;
+
+public class MultiProviderTest {
+
+    private static final String METAINFO = "META-INF/services/FooService";
+    private static String COMBO_CP = Utils.TEST_CLASS_PATH + File.pathSeparator;
+    private static String TEST_CLASS_PATH = System.getProperty("test.classes", ".");
+    private static boolean signJars = false;
+    static final int NUM_JARS = 5;
+
+
+    private static final String KEYSTORE = "keystore.jks";
+    private static final String ALIAS = "JavaTest";
+    private static final String STOREPASS = "changeit";
+    private static final String KEYPASS = "changeit";
+
+    public static void main(String[] args) throws Throwable {
+        signJars = args.length >=1 && args[0].equals("sign");
+        initialize();
+        List<String> cmds = new ArrayList<>();
+        cmds.add(JDKToolFinder.getJDKTool("java"));
+        cmds.addAll(asList(Utils.getTestJavaOpts()));
+        cmds.addAll(List.of(
+                "-cp",
+                COMBO_CP,
+                "--add-opens",
+                "java.base/jdk.internal.access=ALL-UNNAMED",
+                "-Djava.util.logging.config.file=" +
+                Path.of(System.getProperty("test.src", "."), "logging.properties").toString(),
+                "MultiThreadLoad",
+                TEST_CLASS_PATH));
+
+        try {
+            OutputAnalyzer outputAnalyzer = ProcessTools.executeCommand(cmds.stream()
+                    .filter(t -> !t.isEmpty())
+                    .toArray(String[]::new))
+                    .shouldHaveExitValue(0);
+            System.out.println("Output:" + outputAnalyzer.getOutput());
+        } catch (Throwable t) {
+            throw new RuntimeException("Unexpected fail.", t);
+        }
+    }
+
+    public static void initialize() throws Throwable {
+        if (signJars) {
+            genKey();
+        }
+        for (int i = 0; i < NUM_JARS; i++) {
+            String p = "FooProvider" + i;
+            String jarName = "FooProvider" + i + ".jar";
+            Path javaPath = Path.of(p + ".java");
+            Path jarPath = Path.of(p + ".jar");
+            String contents = "public class FooProvider" + i + " extends FooService { }";
+            Files.write(javaPath, contents.getBytes());
+            CompilerUtils.compile(javaPath, Path.of(System.getProperty("test.classes")), "-cp", Utils.TEST_CLASS_PATH);
+            createJar(jarPath, p, List.of(p));
+            if (signJars) {
+                signJar(TEST_CLASS_PATH + File.separator + jarName);
+            }
+            COMBO_CP += TEST_CLASS_PATH + File.separator + jarName + File.pathSeparator;
+        }
+    }
+
+    private static void createProviderConfig(Path config, String providerName) throws Exception {
+        Files.createDirectories(config.getParent());
+        Files.write(config, providerName.getBytes(), CREATE);
+    }
+
+    private static void createJar(Path jar, String provider, List<String> files) throws Exception {
+        Path xdir = Path.of(provider);
+        createProviderConfig(xdir.resolve(METAINFO), provider);
+
+        for (String f : files) {
+            Path source = Path.of(Utils.TEST_CLASSES, f + ".class");
+            Path target = xdir.resolve(source.getFileName());
+            Files.copy(source, target, REPLACE_EXISTING);
+        }
+        JarUtils.createJarFile(Path.of(TEST_CLASS_PATH, jar.getFileName().toString()), xdir);
+    }
+
+    private static void genKey() throws Throwable {
+        String keytool = JDKToolFinder.getJDKTool("keytool");
+        Files.deleteIfExists(Paths.get(KEYSTORE));
+        ProcessTools.executeCommand(keytool,
+                "-J-Duser.language=en",
+                "-J-Duser.country=US",
+                "-genkey",
+                "-keyalg", "rsa",
+                "-alias", ALIAS,
+                "-keystore", KEYSTORE,
+                "-keypass", KEYPASS,
+                "-dname", "cn=sample",
+                "-storepass", STOREPASS
+        ).shouldHaveExitValue(0);
+    }
+
+
+    private static OutputAnalyzer signJar(String jarName) throws Throwable {
+        List<String> args = new ArrayList<>();
+        args.add("-verbose");
+        args.add(jarName);
+        args.add(ALIAS);
+
+        return jarsigner(args);
+    }
+
+    private static OutputAnalyzer jarsigner(List<String> extra)
+            throws Throwable {
+        JDKToolLauncher launcher = JDKToolLauncher.createUsingTestJDK("jarsigner")
+                .addVMArg("-Duser.language=en")
+                .addVMArg("-Duser.country=US")
+                .addToolArg("-keystore")
+                .addToolArg(KEYSTORE)
+                .addToolArg("-storepass")
+                .addToolArg(STOREPASS)
+                .addToolArg("-keypass")
+                .addToolArg(KEYPASS);
+        for (String s : extra) {
+            if (s.startsWith("-J")) {
+                launcher.addVMArg(s.substring(2));
+            } else {
+                launcher.addToolArg(s);
+            }
+        }
+        return ProcessTools.executeCommand(launcher.getCommand());
+    }
+
+}
+
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/jar/JarFile/jarVerification/MultiThreadLoad.java	Mon Jan 13 21:16:27 2020 +0000
@@ -0,0 +1,128 @@
+/*
+ * Copyright (c) 2020, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import jdk.internal.access.JavaUtilJarAccess;
+import jdk.internal.access.SharedSecrets;
+
+import java.io.*;
+import java.nio.file.Path;
+import java.util.*;
+import java.util.concurrent.CountDownLatch;
+import java.util.jar.JarFile;
+
+public class MultiThreadLoad {
+
+    private static PrintStream out = System.err;
+    static String TEST_CLASS_PATH;
+    private static final JavaUtilJarAccess JUJA = SharedSecrets.javaUtilJarAccess();
+    private static CountDownLatch cdl = new CountDownLatch(1);
+
+    private static <T> Set<T> setOf(Iterable<T> it) {
+        Set<T> s = new HashSet<T>();
+        for (T t : it)
+            s.add(t);
+        return s;
+    }
+
+    private static <T> void checkEquals(Set<T> s1, Set<T> s2, boolean eq) {
+        if (s1.equals(s2) != eq)
+            throw new RuntimeException(String.format("%b %s : %s",
+                                                     eq, s1, s2));
+    }
+
+    abstract static class TestLoader {
+        String name;
+
+        TestLoader(String name) { this.name = name; }
+
+        abstract ServiceLoader<FooService> load();
+    }
+
+    static TestLoader tcclLoader = new TestLoader("Thread context class loader") {
+        ServiceLoader<FooService> load() {
+            return ServiceLoader.load(FooService.class);
+        }
+    };
+
+    static TestLoader systemClLoader = new TestLoader("System class loader") {
+        ServiceLoader<FooService> load() {
+            return ServiceLoader.load(FooService.class, ClassLoader.getSystemClassLoader());
+        }
+    };
+
+    static TestLoader nullClLoader = new TestLoader("null (defer to system class loader)") {
+        ServiceLoader<FooService> load() {
+            return ServiceLoader.load(FooService.class, null);
+        }
+    };
+
+    public static void main(String[] args) {
+        // keep reference to variables for the newly launced process
+        TEST_CLASS_PATH = args[0];
+        for (TestLoader tl : Arrays.asList(tcclLoader, systemClLoader, nullClLoader)) {
+            test(tl);
+        }
+    }
+
+    static void test(TestLoader tl) {
+        Runnable r1 = () -> {
+            ServiceLoader<FooService> sl = tl.load();
+            out.format("%s: %s%n", tl.name, sl);
+
+            // Providers are cached
+            Set<FooService> ps = setOf(sl);
+            cdl.countDown();
+            checkEquals(ps, setOf(sl), true);
+
+            // The cache can be flushed and reloaded
+            sl.reload();
+            checkEquals(ps, setOf(sl), false);
+        };
+
+        Runnable r2 = () -> {
+            jarCrawler(Path.of(TEST_CLASS_PATH));
+        };
+        new Thread(r2).start();
+        new Thread(r1).start();
+
+    }
+
+    private static void jarCrawler(Path p) {
+        try {
+            // let the other thread spin up
+            cdl.await();
+        } catch (InterruptedException e) {
+            // ignore
+        }
+        try {
+            for (int i = MultiProviderTest.NUM_JARS -1; i >= 0; i--) {
+                JUJA.ensureInitialization(new JarFile(TEST_CLASS_PATH + File.separator
+                        + "FooProvider" + i + ".jar"));
+            }
+        } catch (Exception e) {
+            System.out.println("Exception during jar crawl: ");
+            e.printStackTrace(System.out);
+        }
+
+    }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/jar/JarFile/jarVerification/logging.properties	Mon Jan 13 21:16:27 2020 +0000
@@ -0,0 +1,13 @@
+############################################################
+#  	Configuration file for log testing
+#
+############################################################
+
+handlers= java.util.logging.ConsoleHandler
+
+.level= FINE
+
+java.util.logging.ConsoleHandler.level = FINE
+java.util.logging.ConsoleHandler.formatter = java.util.logging.SimpleFormatter
+
+jdk.event.security.level = FINE