# HG changeset patch
# User Doug Simon <doug.simon@oracle.com>
# Date 1500486251 -7200
# Node ID f4e6ddeb5b6fd532b2616116d9846b6c95969e57
# Parent  28d2f9c907b4554b752ebbfe882bbedefc55c0f0
race in field updates when creating ArrayKlasses can lead to crash (JDK-8182397)

diff -r 28d2f9c907b4 -r f4e6ddeb5b6f src/share/vm/classfile/javaClasses.cpp
--- a/src/share/vm/classfile/javaClasses.cpp	Mon Jul 10 12:38:12 2017 +0200
+++ b/src/share/vm/classfile/javaClasses.cpp	Wed Jul 19 19:44:11 2017 +0200
@@ -589,6 +589,7 @@
   if (SystemDictionary::Class_klass_loaded()) {
     // Allocate mirror (java.lang.Class instance)
     Handle mirror = InstanceMirrorKlass::cast(SystemDictionary::Class_klass())->allocate_instance(k, CHECK);
+    Handle comp_mirror;
 
     // Setup indirection from mirror->klass
     if (!k.is_null()) {
@@ -602,21 +603,21 @@
 
     // It might also have a component mirror.  This mirror must already exist.
     if (k->oop_is_array()) {
-      Handle comp_mirror;
       if (k->oop_is_typeArray()) {
         BasicType type = TypeArrayKlass::cast(k())->element_type();
-        comp_mirror = Universe::java_mirror(type);
+        comp_mirror = Handle(THREAD, Universe::java_mirror(type));
       } else {
         assert(k->oop_is_objArray(), "Must be");
         Klass* element_klass = ObjArrayKlass::cast(k())->element_klass();
         assert(element_klass != NULL, "Must have an element klass");
-        comp_mirror = element_klass->java_mirror();
+        comp_mirror = Handle(THREAD, element_klass->java_mirror());
       }
       assert(comp_mirror.not_null(), "must have a mirror");
 
       // Two-way link between the array klass and its component mirror:
       ArrayKlass::cast(k())->set_component_mirror(comp_mirror());
-      set_array_klass(comp_mirror(), k());
+      // Set after k->java_mirror() is published.
+      // set_array_klass(comp_mirror(), k());
     } else {
       assert(k->oop_is_instance(), "Must be");
 
@@ -635,10 +636,11 @@
     assert(class_loader() == k->class_loader(), "should be same");
     set_class_loader(mirror(), class_loader());
 
-    // Setup indirection from klass->mirror last
+    // Setup indirection from klass->mirror
     // after any exceptions can happen during allocations.
-    if (!k.is_null()) {
-      k->set_java_mirror(mirror());
+    k->set_java_mirror(mirror());
+    if (comp_mirror() != NULL) {
+        set_array_klass(comp_mirror(), k());
     }
   } else {
     if (fixup_mirror_list() == NULL) {
@@ -811,10 +813,9 @@
   return k;
 }
 
-
 void java_lang_Class::set_array_klass(oop java_class, Klass* klass) {
   assert(klass->is_klass() && klass->oop_is_array(), "should be array klass");
-  java_class->metadata_field_put(_array_klass_offset, klass);
+  java_class->metadata_field_put_volatile(_array_klass_offset, klass);
 }
 
 
diff -r 28d2f9c907b4 -r f4e6ddeb5b6f src/share/vm/oops/oop.hpp
--- a/src/share/vm/oops/oop.hpp	Mon Jul 10 12:38:12 2017 +0200
+++ b/src/share/vm/oops/oop.hpp	Wed Jul 19 19:44:11 2017 +0200
@@ -199,6 +199,7 @@
 
   Metadata* metadata_field(int offset) const;
   void metadata_field_put(int offset, Metadata* value);
+  void metadata_field_put_volatile(int offset, Metadata* value);
 
   jbyte byte_field(int offset) const;
   void byte_field_put(int offset, jbyte contents);
diff -r 28d2f9c907b4 -r f4e6ddeb5b6f src/share/vm/oops/oop.inline.hpp
--- a/src/share/vm/oops/oop.inline.hpp	Mon Jul 10 12:38:12 2017 +0200
+++ b/src/share/vm/oops/oop.inline.hpp	Wed Jul 19 19:44:11 2017 +0200
@@ -324,6 +324,12 @@
   *metadata_field_addr(offset) = value;
 }
 
+inline void oopDesc::metadata_field_put_volatile(int offset, Metadata* value) {
+  OrderAccess::release();
+  *metadata_field_addr(offset) = value;
+  OrderAccess::fence();
+}
+
 inline void oopDesc::obj_field_put_raw(int offset, oop value) {
   UseCompressedOops ?
     encode_store_heap_oop(obj_field_addr<narrowOop>(offset), value) :
diff -r 28d2f9c907b4 -r f4e6ddeb5b6f test/runtime/CreateMirror/ArraysNewInstanceBug.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/runtime/CreateMirror/ArraysNewInstanceBug.java	Wed Jul 19 19:44:11 2017 +0200
@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2017, 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 ArraysNewInstanceBug
+ * @bug 8182397
+ * @summary race in setting array_klass field for component mirror with mirror update for klass
+ * @run main/othervm -Xcomp ArraysNewInstanceBug
+ */
+
+// This test crashes in compiled code with race, because the compiler generates code that assumes this ordering.
+import java.lang.reflect.Array;
+import java.net.URL;
+import java.net.URLClassLoader;
+
+public class ArraysNewInstanceBug implements Runnable {
+    static Class<?>[] classes;
+
+    int start;
+
+    ArraysNewInstanceBug(int start) {
+        this.start = start;
+    }
+
+    String[] result;
+
+    public void run() {
+        result = new String[classes.length];
+        System.err.print('.');
+        for (int i = start; i < classes.length; i++) {
+            result[i] = Array.newInstance(classes[i], 0).getClass().getName();
+        }
+    }
+
+    public static void main(String[] args) throws Throwable {
+        Class<?> c = ArraysNewInstanceBug.class;
+        ClassLoader apploader =  c.getClassLoader();
+        for (int iter = 0; iter < 10 ; iter++) {  // 10 is enough to get it to crash on my machine.
+            System.err.print('[');
+            classes = new Class<?>[1000];
+            String urlpath = "file://" + System.getProperty("test.classes") + "/";
+            for (int i = 0; i < classes.length; i++) {
+                ClassLoader loader = new URLClassLoader(new URL[] { new URL(urlpath) }, apploader.getParent());
+                classes[i] = loader.loadClass(c.getSimpleName());
+            }
+            System.err.print(']');
+            System.err.print('(');
+            int threadCount = 64;
+            Thread[] threads = new Thread[threadCount];
+            for (int i = 0; i < threads.length; i++) {
+                threads[i] = new Thread(new ArraysNewInstanceBug(i));
+            }
+            for (int i = 0; i < threads.length; i++) {
+                threads[i].start();
+            }
+            for (int i = 0; i < threads.length; i++) {
+                threads[i].join();
+            }
+            System.err.print(')');
+        }
+    }
+}