changeset 10980:7c1797994c29

8064846: Lazy-init thread safety problems in core reflection Summary: Make several fields in core reflection volatile Reviewed-by: jfranck, shade, plevart
author martin
date Fri, 14 Nov 2014 11:23:07 -0800
parents e9cc780faafd
children a4c328a2065d
files src/java.base/share/classes/sun/reflect/generics/repository/ClassRepository.java src/java.base/share/classes/sun/reflect/generics/repository/GenericDeclRepository.java src/java.base/share/classes/sun/reflect/generics/scope/AbstractScope.java
diffstat 3 files changed, 30 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/sun/reflect/generics/repository/ClassRepository.java	Mon Nov 17 15:30:22 2014 +0300
+++ b/src/java.base/share/classes/sun/reflect/generics/repository/ClassRepository.java	Fri Nov 14 11:23:07 2014 -0800
@@ -42,8 +42,11 @@
 
     public static final ClassRepository NONE = ClassRepository.make("Ljava/lang/Object;", null);
 
-    private Type superclass; // caches the generic superclass info
-    private Type[] superInterfaces; // caches the generic superinterface info
+    /** The generic superclass info.  Lazily initialized. */
+    private volatile Type superclass;
+
+    /** The generic superinterface info.  Lazily initialized. */
+    private volatile Type[] superInterfaces;
 
  // private, to enforce use of static factory
     private ClassRepository(String rawSig, GenericsFactory f) {
@@ -79,31 +82,34 @@
  * with which the repository was created.
  */
 
-    public Type getSuperclass(){
+    public Type getSuperclass() {
+        Type superclass = this.superclass;
         if (superclass == null) { // lazily initialize superclass
             Reifier r = getReifier(); // obtain visitor
             // Extract superclass subtree from AST and reify
             getTree().getSuperclass().accept(r);
             // extract result from visitor and cache it
             superclass = r.getResult();
-            }
+            this.superclass = superclass;
+        }
         return superclass; // return cached result
     }
 
-    public Type[] getSuperInterfaces(){
+    public Type[] getSuperInterfaces() {
+        Type[] superInterfaces = this.superInterfaces;
         if (superInterfaces == null) { // lazily initialize super interfaces
             // first, extract super interface subtree(s) from AST
             TypeTree[] ts  = getTree().getSuperInterfaces();
             // create array to store reified subtree(s)
-            Type[] sis = new Type[ts.length];
+            superInterfaces = new Type[ts.length];
             // reify all subtrees
             for (int i = 0; i < ts.length; i++) {
                 Reifier r = getReifier(); // obtain visitor
                 ts[i].accept(r);// reify subtree
                 // extract result from visitor and store it
-                sis[i] = r.getResult();
+                superInterfaces[i] = r.getResult();
             }
-            superInterfaces = sis; // cache overall result
+            this.superInterfaces = superInterfaces;
         }
         return superInterfaces.clone(); // return cached result
     }
--- a/src/java.base/share/classes/sun/reflect/generics/repository/GenericDeclRepository.java	Mon Nov 17 15:30:22 2014 +0300
+++ b/src/java.base/share/classes/sun/reflect/generics/repository/GenericDeclRepository.java	Fri Nov 14 11:23:07 2014 -0800
@@ -42,7 +42,8 @@
 public abstract class GenericDeclRepository<S extends Signature>
     extends AbstractRepository<S> {
 
-    private TypeVariable<?>[] typeParams; // caches the formal type parameters
+    /** The formal type parameters.  Lazily initialized. */
+    private volatile TypeVariable<?>[] typeParams;
 
     protected GenericDeclRepository(String rawSig, GenericsFactory f) {
         super(rawSig, f);
@@ -55,8 +56,7 @@
  * If the corresponding field is non-null, it is returned.
  * If not, it is created lazily. This is done by selecting the appropriate
  * part of the tree and transforming it into a reflective object
- * using a visitor.
- * a visitor, which is created by feeding it the factory
+ * using a visitor, which is created by feeding it the factory
  * with which the repository was created.
  */
 
@@ -64,20 +64,21 @@
      * Return the formal type parameters of this generic declaration.
      * @return the formal type parameters of this generic declaration
      */
-    public TypeVariable<?>[] getTypeParameters(){
+    public TypeVariable<?>[] getTypeParameters() {
+        TypeVariable<?>[] typeParams = this.typeParams;
         if (typeParams == null) { // lazily initialize type parameters
             // first, extract type parameter subtree(s) from AST
             FormalTypeParameter[] ftps = getTree().getFormalTypeParameters();
             // create array to store reified subtree(s)
-            TypeVariable<?>[] tps = new TypeVariable<?>[ftps.length];
+            typeParams = new TypeVariable<?>[ftps.length];
             // reify all subtrees
             for (int i = 0; i < ftps.length; i++) {
                 Reifier r = getReifier(); // obtain visitor
                 ftps[i].accept(r); // reify subtree
                 // extract result from visitor and store it
-                tps[i] = (TypeVariable<?>) r.getResult();
+                typeParams[i] = (TypeVariable<?>) r.getResult();
             }
-            typeParams = tps; // cache overall result
+            this.typeParams = typeParams; // cache overall result
         }
         return typeParams.clone(); // return cached result
     }
--- a/src/java.base/share/classes/sun/reflect/generics/scope/AbstractScope.java	Mon Nov 17 15:30:22 2014 +0300
+++ b/src/java.base/share/classes/sun/reflect/generics/scope/AbstractScope.java	Fri Nov 14 11:23:07 2014 -0800
@@ -42,7 +42,9 @@
     implements Scope {
 
     private final D recvr; // the declaration whose scope this instance represents
-    private Scope enclosingScope; // the enclosing scope of this scope
+
+    /** The enclosing scope of this scope.  Lazily initialized. */
+    private volatile Scope enclosingScope;
 
     /**
      * Constructor. Takes a reflective object whose scope the newly
@@ -71,7 +73,11 @@
      * @return the enclosing scope
      */
     protected Scope getEnclosingScope(){
-        if (enclosingScope == null) {enclosingScope = computeEnclosingScope();}
+        Scope enclosingScope = this.enclosingScope;
+        if (enclosingScope == null) {
+            enclosingScope = computeEnclosingScope();
+            this.enclosingScope = enclosingScope;
+        }
         return enclosingScope;
     }