8156584: Initialization race in sun.security.x509.AlgorithmId.get
authormartin
Thu, 12 May 2016 18:57:24 -0700
changeset 37908 b9bb960b959c
parent 37907 643c10927a1a
child 37910 30c530404433
8156584: Initialization race in sun.security.x509.AlgorithmId.get Summary: Use safe variant of double-checked locking to initialize oidTable Reviewed-by: xuelei
jdk/src/java.base/share/classes/sun/security/x509/AlgorithmId.java
jdk/test/sun/security/x509/AlgorithmId/OidTableInit.java
--- a/jdk/src/java.base/share/classes/sun/security/x509/AlgorithmId.java	Fri May 13 18:58:32 2016 +0000
+++ b/jdk/src/java.base/share/classes/sun/security/x509/AlgorithmId.java	Thu May 12 18:57:24 2016 -0700
@@ -552,58 +552,61 @@
             return AlgorithmId.sha512WithECDSA_oid;
         }
 
-        // See if any of the installed providers supply a mapping from
-        // the given algorithm name to an OID string
-        String oidString;
-        if (!initOidTable) {
-            Provider[] provs = Security.getProviders();
-            for (int i=0; i<provs.length; i++) {
-                for (Enumeration<Object> enum_ = provs[i].keys();
-                     enum_.hasMoreElements(); ) {
-                    String alias = (String)enum_.nextElement();
-                    String upperCaseAlias = alias.toUpperCase(Locale.ENGLISH);
-                    int index;
-                    if (upperCaseAlias.startsWith("ALG.ALIAS") &&
-                            (index=upperCaseAlias.indexOf("OID.", 0)) != -1) {
-                        index += "OID.".length();
-                        if (index == alias.length()) {
-                            // invalid alias entry
-                            break;
-                        }
-                        if (oidTable == null) {
-                            oidTable = new HashMap<>();
-                        }
-                        oidString = alias.substring(index);
-                        String stdAlgName = provs[i].getProperty(alias);
-                        if (stdAlgName != null) {
-                            stdAlgName = stdAlgName.toUpperCase(Locale.ENGLISH);
-                        }
-                        if (stdAlgName != null &&
-                                oidTable.get(stdAlgName) == null) {
-                            oidTable.put(stdAlgName,
-                                         new ObjectIdentifier(oidString));
-                        }
-                    }
-                }
-            }
-
-            if (oidTable == null) {
-                oidTable = Collections.<String,ObjectIdentifier>emptyMap();
-            }
-            initOidTable = true;
-        }
-
-        return oidTable.get(name.toUpperCase(Locale.ENGLISH));
+        return oidTable().get(name.toUpperCase(Locale.ENGLISH));
     }
 
     private static ObjectIdentifier oid(int ... values) {
         return ObjectIdentifier.newInternal(values);
     }
 
-    private static boolean initOidTable = false;
-    private static Map<String,ObjectIdentifier> oidTable;
+    private static volatile Map<String,ObjectIdentifier> oidTable;
     private static final Map<ObjectIdentifier,String> nameTable;
 
+    /** Returns the oidTable, lazily initializing it on first access. */
+    private static Map<String,ObjectIdentifier> oidTable()
+        throws IOException {
+        // Double checked locking; safe because oidTable is volatile
+        Map<String,ObjectIdentifier> tab;
+        if ((tab = oidTable) == null) {
+            synchronized (AlgorithmId.class) {
+                if ((tab = oidTable) == null)
+                    oidTable = tab = computeOidTable();
+            }
+        }
+        return tab;
+    }
+
+    /** Collects the algorithm names from the installed providers. */
+    private static HashMap<String,ObjectIdentifier> computeOidTable()
+        throws IOException {
+        HashMap<String,ObjectIdentifier> tab = new HashMap<>();
+        for (Provider provider : Security.getProviders()) {
+            for (Object key : provider.keySet()) {
+                String alias = (String)key;
+                String upperCaseAlias = alias.toUpperCase(Locale.ENGLISH);
+                int index;
+                if (upperCaseAlias.startsWith("ALG.ALIAS") &&
+                    (index=upperCaseAlias.indexOf("OID.", 0)) != -1) {
+                    index += "OID.".length();
+                    if (index == alias.length()) {
+                        // invalid alias entry
+                        break;
+                    }
+                    String oidString = alias.substring(index);
+                    String stdAlgName = provider.getProperty(alias);
+                    if (stdAlgName != null) {
+                        stdAlgName = stdAlgName.toUpperCase(Locale.ENGLISH);
+                    }
+                    if (stdAlgName != null &&
+                        tab.get(stdAlgName) == null) {
+                        tab.put(stdAlgName, new ObjectIdentifier(oidString));
+                    }
+                }
+            }
+        }
+        return tab;
+    }
+
     /*****************************************************************/
 
     /*
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/sun/security/x509/AlgorithmId/OidTableInit.java	Thu May 12 18:57:24 2016 -0700
@@ -0,0 +1,77 @@
+/*
+ * Copyright (c) 2016 Google Inc. 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 8156584
+ * @modules java.base/sun.security.x509
+ * @summary AlgorithmId.get initialization thread safety
+ * @run main/othervm OidTableInit
+ */
+
+import java.util.ArrayList;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import sun.security.x509.AlgorithmId;
+
+public class OidTableInit {
+    public static void main(String[] args) throws Throwable {
+        final String[] algorithmNames = {
+            "PBKDF2WITHHMACSHA1",
+            "PBEWITHMD5ANDDES",
+            "DSA",
+            "SHA384WITHRSA",
+            "RSA",
+            "SHA1WITHDSA",
+            "SHA512WITHRSA",
+            "MD2WITHRSA",
+            "PBEWITHSHA1ANDDESEDE",
+            "SHA1WITHRSA",
+            "DIFFIEHELLMAN",
+            "MD5WITHRSA",
+            "PBEWITHSHA1ANDRC2_40",
+            "SHA256WITHRSA",
+        };
+
+        final int THREADS = 2;
+        final ExecutorService pool = Executors.newFixedThreadPool(THREADS);
+        final CountDownLatch startingGate = new CountDownLatch(THREADS);
+        final Runnable r = new Runnable() { public void run() {
+            startingGate.countDown();
+            do {} while (startingGate.getCount() > 0);
+            try {
+                for (String algorithmName : algorithmNames)
+                    AlgorithmId.get(algorithmName);
+            } catch (Throwable fail) {
+                throw new AssertionError(fail);
+            }
+        }};
+        final ArrayList<Future<?>> futures = new ArrayList<>();
+        for (int i = 0; i < THREADS; i++)
+            futures.add(pool.submit(r));
+        pool.shutdown();
+        for (Future<?> future : futures) future.get();
+    }
+}