openwebbeans-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dblev...@apache.org
Subject svn commit: r1481426 - in /openwebbeans/trunk/webbeans-impl/src: main/java/org/apache/webbeans/portable/AnnotatedTypeImpl.java test/java/org/apache/webbeans/portable/AnnotatedTypeImplTest.java
Date Sat, 11 May 2013 21:57:16 GMT
Author: dblevins
Date: Sat May 11 21:57:16 2013
New Revision: 1481426

URL: http://svn.apache.org/r1481426
Log:
OWB-858: AnnotatedTypeImpl not thread safe
Better implementation which uses double-check locking and also reduces memory requirements
for the lazy case by 128 bits

Modified:
    openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/portable/AnnotatedTypeImpl.java
    openwebbeans/trunk/webbeans-impl/src/test/java/org/apache/webbeans/portable/AnnotatedTypeImplTest.java

Modified: openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/portable/AnnotatedTypeImpl.java
URL: http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/portable/AnnotatedTypeImpl.java?rev=1481426&r1=1481425&r2=1481426&view=diff
==============================================================================
--- openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/portable/AnnotatedTypeImpl.java
(original)
+++ openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/portable/AnnotatedTypeImpl.java
Sat May 11 21:57:16 2013
@@ -28,10 +28,6 @@ import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.FutureTask;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 import javax.enterprise.inject.spi.AnnotatedConstructor;
 import javax.enterprise.inject.spi.AnnotatedField;
@@ -61,33 +57,7 @@ class AnnotatedTypeImpl<X>
      */
     private final Class<X> annotatedClass;
 
-    /**
-     * Constructors
-     */
-    private Set<AnnotatedConstructor<X>> constructors = null;
-
-    /**
-     * Fields
-     */
-    private Set<AnnotatedField<? super X>> fields = null;
-
-    /**
-     * Methods
-     */
-    private Set<AnnotatedMethod<? super X>> methods = null;
-
-    private final AtomicBoolean notInitialized = new AtomicBoolean(true);
-
-    private final FutureTask initializer = new FutureTask(new Callable()
-    {
-        public Object call()
-            throws Exception
-        {
-            init();
-            return null;
-        }
-    });
-
+    private volatile State state;
 
     /**
      * Creates a new instance.
@@ -126,72 +96,6 @@ class AnnotatedTypeImpl<X>
         }
     }
 
-    private synchronized void init()
-    {
-        if (constructors == null)
-        {
-            constructors = new HashSet<AnnotatedConstructor<X>>();
-            fields = new HashSet<AnnotatedField<? super X>>();
-            methods = new HashSet<AnnotatedMethod<? super X>>();
-
-            Constructor<?>[] decCtxs =
-                getWebBeansContext().getSecurityService().doPrivilegedGetDeclaredConstructors(annotatedClass);
-
-            for (Constructor<?> ct : decCtxs)
-            {
-                if (!ct.isSynthetic())
-                {
-                    AnnotatedConstructor<X> ac =
-                        new AnnotatedConstructorImpl<X>(getWebBeansContext(), (Constructor<X>)
ct, this);
-                    constructors.add(ac);
-                }
-            }
-            if (constructors.isEmpty())
-            {
-                // must be implicit default constructor
-                Constructor<X> constructor =
-                    getWebBeansContext().getSecurityService().doPrivilegedGetDeclaredConstructor(annotatedClass);
-                if (constructor != null)
-                {
-                    constructors.add(new AnnotatedConstructorImpl<X>(getWebBeansContext(),
constructor, this));
-                }
-            }
-
-            Field[] decFields = getWebBeansContext().getSecurityService().doPrivilegedGetDeclaredFields(annotatedClass);
-            Method[] decMethods =
-                getWebBeansContext().getSecurityService().doPrivilegedGetDeclaredMethods(annotatedClass);
-            for (Field f : decFields)
-            {
-                if (!f.isSynthetic())
-                {
-                    AnnotatedField<X> af = new AnnotatedFieldImpl<X>(getWebBeansContext(),
f, this);
-                    fields.add(af);
-                }
-            }
-
-            for (Method m : decMethods)
-            {
-                if (!m.isSynthetic() && !m.isBridge())
-                {
-                    AnnotatedMethod<X> am = new AnnotatedMethodImpl<X>(getWebBeansContext(),
m, this);
-                    methods.add(am);
-                }
-            }
-
-            if (supertype != null)
-            {
-                fields.addAll(supertype.getFields());
-                for (AnnotatedMethod<? super X> method : supertype.getMethods())
-                {
-                    if (!isOverridden(method))
-                    {
-                        methods.add(method);
-                    }
-                }
-            }
-        }
-    }
-
     /**
      * {@inheritDoc}
      */
@@ -209,43 +113,7 @@ class AnnotatedTypeImpl<X>
      */
     void addAnnotatedConstructor(AnnotatedConstructor<X> constructor)
     {
-        ensureInitialized();
-        constructors.add(constructor);
-    }
-
-    private void ensureInitialized()
-    {
-        try
-        {
-            if (notInitialized.get())
-            {
-                do
-                {
-                    // If this thread is the one to set the state to "notInitialized=false",
-                    // then this thread is also responsible for calling the initializer
-                    if (notInitialized.compareAndSet(true, false))
-                    {
-                        initializer.run();
-                    }
-
-                    // Try again.
-                }
-                while (notInitialized.get());
-            }
-
-            // This is the magic blocking call that protects our read access
-            // The 'get' call will not return until the initializer has finished running
-            initializer.get();
-        }
-        catch (InterruptedException e)
-        {
-            Thread.interrupted();
-            throw new IllegalStateException("Lazy Initialization of AnnotatedType failed.",
e);
-        }
-        catch (ExecutionException e)
-        {
-            throw new IllegalStateException("Lazy Initialization of AnnotatedType failed.",
e);
-        }
+        getState().constructors.add(constructor);
     }
 
     /**
@@ -255,8 +123,7 @@ class AnnotatedTypeImpl<X>
      */
     void addAnnotatedField(AnnotatedField<? super X> field)
     {
-        ensureInitialized();
-        fields.add(field);
+        getState().fields.add(field);
     }
 
     /**
@@ -266,8 +133,7 @@ class AnnotatedTypeImpl<X>
      */
     void addAnnotatedMethod(AnnotatedMethod<? super X> method)
     {
-        ensureInitialized();
-        methods.add(method);
+        getState().methods.add(method);
     }
 
     /**
@@ -276,8 +142,7 @@ class AnnotatedTypeImpl<X>
     @Override
     public Set<AnnotatedConstructor<X>> getConstructors()
     {
-        ensureInitialized();
-        return Collections.unmodifiableSet(constructors);
+        return Collections.unmodifiableSet(getState().constructors);
     }
 
     /**
@@ -286,8 +151,7 @@ class AnnotatedTypeImpl<X>
     @Override
     public Set<AnnotatedField<? super X>> getFields()
     {
-        ensureInitialized();
-        return Collections.unmodifiableSet(fields);
+        return Collections.unmodifiableSet(getState().fields);
     }
 
     /**
@@ -296,19 +160,121 @@ class AnnotatedTypeImpl<X>
     @Override
     public Set<AnnotatedMethod<? super X>> getMethods()
     {
-        ensureInitialized();
-        return Collections.unmodifiableSet(methods);
+        return Collections.unmodifiableSet(getState().methods);
+    }
+
+    private State getState()
+    {
+        State result = state;
+        // Double check locking with standard optimization to avoid
+        // extra reads on the volatile field 'state'
+        if (result == null)
+        {
+            synchronized (this)
+            {
+                result = state;
+                if (result == null)
+                {
+                    result = new State();
+                    state = result;
+                }
+            }
+        }
+
+        return result;
     }
 
-    private boolean isOverridden(AnnotatedMethod<? super X> superclassMethod)
+
+    private class State
     {
-        for (AnnotatedMethod<? super X> subclassMethod : methods)
+
+        /**
+         * Constructors
+         */
+        private final Set<AnnotatedConstructor<X>> constructors = new HashSet<AnnotatedConstructor<X>>();
+
+        /**
+         * Fields
+         */
+        private final Set<AnnotatedField<? super X>> fields = new HashSet<AnnotatedField<?
super X>>();
+
+        /**
+         * Methods
+         */
+        private final Set<AnnotatedMethod<? super X>> methods = new HashSet<AnnotatedMethod<?
super X>>();
+
+        private State()
+        {
+            Constructor<?>[] decCtxs =
+                getWebBeansContext().getSecurityService().doPrivilegedGetDeclaredConstructors(annotatedClass);
+
+            for (Constructor<?> ct : decCtxs)
+            {
+                if (!ct.isSynthetic())
+                {
+                    AnnotatedConstructor<X> ac =
+                        new AnnotatedConstructorImpl<X>(getWebBeansContext(), (Constructor<X>)
ct,
+                                                        AnnotatedTypeImpl.this);
+                    constructors.add(ac);
+                }
+            }
+            if (constructors.isEmpty())
+            {
+                // must be implicit default constructor
+                Constructor<X> constructor =
+                    getWebBeansContext().getSecurityService().doPrivilegedGetDeclaredConstructor(annotatedClass);
+                if (constructor != null)
+                {
+                    constructors.add(
+                        new AnnotatedConstructorImpl<X>(getWebBeansContext(), constructor,
AnnotatedTypeImpl.this));
+                }
+            }
+
+            Field[] decFields = getWebBeansContext().getSecurityService().doPrivilegedGetDeclaredFields(annotatedClass);
+            Method[] decMethods =
+                getWebBeansContext().getSecurityService().doPrivilegedGetDeclaredMethods(annotatedClass);
+            for (Field f : decFields)
+            {
+                if (!f.isSynthetic())
+                {
+                    AnnotatedField<X> af = new AnnotatedFieldImpl<X>(getWebBeansContext(),
f, AnnotatedTypeImpl.this);
+                    fields.add(af);
+                }
+            }
+
+            for (Method m : decMethods)
+            {
+                if (!m.isSynthetic() && !m.isBridge())
+                {
+                    AnnotatedMethod<X> am = new AnnotatedMethodImpl<X>(getWebBeansContext(),
m, AnnotatedTypeImpl.this);
+                    methods.add(am);
+                }
+            }
+
+            if (supertype != null)
+            {
+                fields.addAll(supertype.getFields());
+                for (AnnotatedMethod<? super X> method : supertype.getMethods())
+                {
+                    if (!isOverridden(method))
+                    {
+                        methods.add(method);
+                    }
+                }
+            }
+
+        }
+
+        private boolean isOverridden(AnnotatedMethod<? super X> superclassMethod)
         {
-            if (ClassUtil.isOverridden(subclassMethod.getJavaMember(), superclassMethod.getJavaMember()))
+            for (AnnotatedMethod<? super X> subclassMethod : methods)
             {
-                return true;
+                if (ClassUtil.isOverridden(subclassMethod.getJavaMember(), superclassMethod.getJavaMember()))
+                {
+                    return true;
+                }
             }
+            return false;
         }
-        return false;
     }
 }

Modified: openwebbeans/trunk/webbeans-impl/src/test/java/org/apache/webbeans/portable/AnnotatedTypeImplTest.java
URL: http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-impl/src/test/java/org/apache/webbeans/portable/AnnotatedTypeImplTest.java?rev=1481426&r1=1481425&r2=1481426&view=diff
==============================================================================
--- openwebbeans/trunk/webbeans-impl/src/test/java/org/apache/webbeans/portable/AnnotatedTypeImplTest.java
(original)
+++ openwebbeans/trunk/webbeans-impl/src/test/java/org/apache/webbeans/portable/AnnotatedTypeImplTest.java
Sat May 11 21:57:16 2013
@@ -78,7 +78,7 @@ public class AnnotatedTypeImplTest
 
         startingPistol.countDown();
 
-        assertTrue("Not all threads finished.", finishLine.await(30, TimeUnit.SECONDS));
+        assertTrue("Not all threads finished.", finishLine.await(1, TimeUnit.MINUTES));
 
         assertEquals(0, exceptions.get());
     }



Mime
View raw message