[seam-commits] Seam SVN: r14973 - in branches/community/Seam_2_2/src: test/integration/src/org/jboss/seam/test/integration and 1 other directory.

seam-commits at lists.jboss.org seam-commits at lists.jboss.org
Tue Jul 3 04:57:59 EDT 2012


Author: manaRH
Date: 2012-07-03 04:57:58 -0400 (Tue, 03 Jul 2012)
New Revision: 14973

Added:
   branches/community/Seam_2_2/src/test/integration/src/org/jboss/seam/test/integration/FactoryLockTest.java
Modified:
   branches/community/Seam_2_2/src/main/org/jboss/seam/Component.java
   branches/community/Seam_2_2/src/test/integration/src/org/jboss/seam/test/integration/ConcurrentFactoryTest.java
   branches/community/Seam_2_2/src/test/integration/src/org/jboss/seam/test/integration/testng.xml
Log:
backported JBSEAM-4993, JBSEAM-4861 to Seam 2.2

Modified: branches/community/Seam_2_2/src/main/org/jboss/seam/Component.java
===================================================================
--- branches/community/Seam_2_2/src/main/org/jboss/seam/Component.java	2012-07-02 12:59:07 UTC (rev 14972)
+++ branches/community/Seam_2_2/src/main/org/jboss/seam/Component.java	2012-07-03 08:57:58 UTC (rev 14973)
@@ -50,7 +50,6 @@
 import java.util.SortedSet;
 import java.util.TreeMap;
 import java.util.TreeSet;
-import java.util.concurrent.locks.ReentrantLock;
 
 import javassist.util.proxy.MethodFilter;
 import javassist.util.proxy.MethodHandler;
@@ -130,8 +129,6 @@
 
    private static final LogProvider log = Logging.getLogProvider(Component.class);
    
-   static ReentrantLock factoryLock = new ReentrantLock();
-   
    private ComponentType type;
    private String name;
    private ScopeType scope;
@@ -2057,7 +2054,7 @@
    private static Object getInstanceFromFactory(String name, ScopeType scope)
    {
       Init init = Init.instance();
-      if (init==null) //for unit tests, yew!
+      if (init == null) // for unit tests, yew!
       {
          return null;
       }
@@ -2066,45 +2063,62 @@
          Init.FactoryMethod factoryMethod = init.getFactory(name);
          Init.FactoryExpression methodBinding = init.getFactoryMethodExpression(name);
          Init.FactoryExpression valueBinding = init.getFactoryValueExpression(name);
-         if ( methodBinding!=null && getOutScope( methodBinding.getScope(), null ).isContextActive() ) //let the XML take precedence
-         {
+         if (methodBinding != null && getOutScope(methodBinding.getScope(), null).isContextActive())
+         {// let the XML take precedence
             Object result = methodBinding.getMethodBinding().invoke();
-            return handleFactoryMethodResult( name, null, result, methodBinding.getScope() );
+            return handleFactoryMethodResult(name, null, result, methodBinding.getScope());
          }
-         else if ( valueBinding!=null && getOutScope( valueBinding.getScope(), null ).isContextActive() ) //let the XML take precedence
-         {
+         else if (valueBinding != null && getOutScope(valueBinding.getScope(), null).isContextActive()) 
+         { // let the XML take precedence
             Object result = valueBinding.getValueBinding().getValue();
-            return handleFactoryMethodResult( name, null, result, valueBinding.getScope() );
+            return handleFactoryMethodResult(name, null, result, valueBinding.getScope());
          }
-         else if ( factoryMethod!=null && getOutScope( factoryMethod.getScope(), factoryMethod.getComponent() ).isContextActive() )
+         else if (factoryMethod != null && getOutScope(factoryMethod.getScope(), factoryMethod.getComponent()).isContextActive())
          {
-            Object factory = Component.getInstance( factoryMethod.getComponent().getName(), true );
-            factoryLock.lock();
-            try
+            Object factory = Component.getInstance(factoryMethod.getComponent().getName(), true);
+            ScopeType scopeResult = getOutScope(factoryMethod.getScope(), factoryMethod.getComponent());
+            ScopeType scopeFactory = factoryMethod.getComponent().getScope();
+            // we need this lock in the following cases: (1) the target scope is
+            // accessed by more than one thread (as we don't want to create the
+            // same object by two threads at the same time for the same scope)
+            // OR (2) the factory is accessed by more than one thread and is
+            // using interceptors (as accessing a factory multiple times might
+            // mess up interceptors). This assumes that (1) the scopes
+            // CONVERSATION, EVENT and PAGE can't be accessed by more than one
+            // thread anyway due to CONVERSATION being locked for the current
+            // thread anyway, and EVENT and PAGE being only short-lived anyway.
+            // (2) a factory that doesn't use injection can be accessed multi
+            // threaded. See JBSEAM-4669/JBSEAM-2419 for the original
+            // discussion.
+            boolean lockingNeeded = ((scopeResult != ScopeType.CONVERSATION && scopeResult != ScopeType.EVENT && scopeResult != ScopeType.PAGE) ||
+                  (scopeFactory != ScopeType.CONVERSATION && scopeFactory != ScopeType.EVENT && scopeFactory != ScopeType.PAGE && factoryMethod.getComponent().interceptionEnabled));
+
+            if (lockingNeeded)
             {
-               // check whether there has been created an instance by another thread while waiting for this function's lock
-               if (scope != STATELESS)
+               // Only one factory instance can access result scope
+               // CONVERSATION / EVENT / PAGE anyway due to
+               // the locking of the conversation.
+               if (scopeResult == ScopeType.CONVERSATION || scopeResult == ScopeType.EVENT || scopeResult == ScopeType.PAGE)
                {
-                  Object value = (scope == null) ? Contexts.lookupInStatefulContexts(name) : scope.getContext().get(name);
-                  if (value != null)
+                  synchronized (factory)
                   {
-                     return value;
+                     return createInstanceFromFactory(name, scope, factoryMethod, factory);
                   }
                }
-               
-               if (factory==null)
-               {
-                  return null;
-               }
+               // synchronize all instances of this factory as they might
+               // outject to the same scope (i.e. factory in EVENT scope,
+               // outjecting to APPLICATION scope).
                else
                {
-                  Object result = factoryMethod.getComponent().callComponentMethod( factory, factoryMethod.getMethod() );
-                  return handleFactoryMethodResult( name, factoryMethod.getComponent(), result, factoryMethod.getScope() );
+                  synchronized (factory.getClass())
+                  {
+                     return createInstanceFromFactory(name, scope, factoryMethod, factory);
+                  }
                }
             }
-            finally 
+            else
             {
-               factoryLock.unlock();
+               return createInstanceFromFactory(name, scope, factoryMethod, factory);
             }
          }
          else
@@ -2114,6 +2128,30 @@
       }
    }
 
+   private static Object createInstanceFromFactory(String name, ScopeType scope, Init.FactoryMethod factoryMethod, Object factory)
+   {
+      // check whether there has been created an instance by another thread
+      // while waiting for this function's lock
+      if (scope != STATELESS)
+      {
+         Object value = (scope == null) ? Contexts.lookupInStatefulContexts(name) : scope.getContext().get(name);
+         if (value != null)
+         {
+            return value;
+         }
+      }
+
+      if (factory == null)
+      {
+         return null;
+      }
+      else
+      {
+         Object result = factoryMethod.getComponent().callComponentMethod(factory, factoryMethod.getMethod());
+         return handleFactoryMethodResult(name, factoryMethod.getComponent(), result, factoryMethod.getScope());
+      }
+   }
+   
    private static Object handleFactoryMethodResult(String name, Component component, Object result, ScopeType scope) {
         // see if a value was outjected by the factory method
         Object value = Contexts.lookupInStatefulContexts(name);
@@ -2163,11 +2201,11 @@
          }
          
       } catch (Exception e) {
-    	  if (getScope()!=STATELESS) {
-    		  getScope().getContext().remove(name); 
-    	  }
+              if (getScope()!=STATELESS) {
+                      getScope().getContext().remove(name); 
+              }
 
-    	  throw new InstantiationException("Could not instantiate Seam component: " + name, e);
+              throw new InstantiationException("Could not instantiate Seam component: " + name, e);
       }
 
       return instance;

Modified: branches/community/Seam_2_2/src/test/integration/src/org/jboss/seam/test/integration/ConcurrentFactoryTest.java
===================================================================
--- branches/community/Seam_2_2/src/test/integration/src/org/jboss/seam/test/integration/ConcurrentFactoryTest.java	2012-07-02 12:59:07 UTC (rev 14972)
+++ branches/community/Seam_2_2/src/test/integration/src/org/jboss/seam/test/integration/ConcurrentFactoryTest.java	2012-07-03 08:57:58 UTC (rev 14973)
@@ -2,74 +2,122 @@
 
 import static org.jboss.seam.ScopeType.APPLICATION;
 
+import java.io.Serializable;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.jboss.seam.annotations.Create;
 import org.jboss.seam.annotations.Factory;
 import org.jboss.seam.annotations.In;
 import org.jboss.seam.annotations.Name;
-import org.jboss.seam.contexts.ServletLifecycle;
+import org.jboss.seam.annotations.Scope;
+import org.jboss.seam.core.Expressions;
 import org.jboss.seam.mock.SeamTest;
-import org.testng.annotations.AfterMethod;
+import org.testng.annotations.AfterClass;
 import org.testng.annotations.Test;
 
+
+// JBSEAM-4669
 public class ConcurrentFactoryTest 
     extends SeamTest 
 {
-    @Override
-    protected void startJbossEmbeddedIfNecessary() 
-          throws org.jboss.deployers.spi.DeploymentException,
-                 java.io.IOException 
+    private volatile boolean exceptionOccured = false;
+    static AtomicInteger testSequence = new AtomicInteger(0);
+
+    private void concurrentFactoryCallTest() throws Exception {
+       new ComponentTest() {
+          @Override
+          protected void testComponents() throws Exception {
+             int myTestSequence;
+             myTestSequence = testSequence.getAndIncrement();
+             if (myTestSequence == 0) {
+                assert "TestString".equals(getValue("#{concurrentFactoryTest.LockHoldingComponent.string}"));
+             } else {
+                try {
+                   Thread.sleep(500);
+                } catch (InterruptedException e) {
+                   e.printStackTrace();
+                }
+                assert "TestString".equals(getValue("#{concurrentFactoryTest.dependentString}"));
+             }
+             System.out.println(myTestSequence);
+
+          }
+      }.run();
+    }
+    
+    private class ConcurrentFactoryCallTestThread extends Thread
     {
-       // don't deploy   
+        public void run() {
+            try
+            {
+                ConcurrentFactoryTest.this.concurrentFactoryCallTest();
+            }
+            catch (Throwable e)
+            {
+                e.printStackTrace();
+                ConcurrentFactoryTest.this.exceptionOccured = true;
+            }
+        };
     }
-
-    @Test(threadPoolSize = 2, invocationCount = 2)
+   
+    @Test(timeOut=10000)
     public void concurrentFactoryCall() 
         throws Exception 
     {
-        new ComponentTest() {
-            @Override
-            protected void testComponents() throws Exception {
-                assert "slowly created String".equals(getValue("#{concurrentFactoryTest.component.injectedString}"));
-            }
-        }.run();
+       Thread thread1 = new ConcurrentFactoryCallTestThread();
+       Thread thread2 = new ConcurrentFactoryCallTestThread();
+
+       thread1.start();
+       thread2.start();
+    
+       thread1.join();
+       thread2.join();
+       
+       assert !exceptionOccured;
     }
     
-    @AfterMethod
-    @Override
-    public void end()
-    {
-       if (session != null) {
-          // Because we run in threads. Only the first thread that finishes ends the session.
-          ServletLifecycle.endSession(session);
+    @Name("concurrentFactoryTest.LockHoldingComponent")
+    @Scope(APPLICATION)
+    static public class LockHoldingComponent implements Serializable {
+       @In(value = "concurrentFactoryTest.slowlyCreatedComponent", create = true) SlowlyCreatedComponent slowlyCreatedComponent;
+       
+       public String getString() {
+          return (String) Expressions.instance().createValueExpression("#{concurrentFactoryTest.plainFactoryGeneratedString}").getValue();
        }
-       session = null;
     }
     
-    @Name("concurrentFactoryTest.component")
-    static public class Component {
-       @In(value = "concurrentFactoryTest.slowlyCreatedString") String injectedString;
-       
-       public String getInjectedString() {
-          return injectedString;
+    @Name("concurrentFactoryTest.slowlyCreatedComponent")
+    static public class SlowlyCreatedComponent {
+       @Create
+       public void slowlyCreate() {
+          try {
+             Thread.sleep(1000);
+          } catch (InterruptedException e) {
+             e.printStackTrace();
+          }
        }
     }
+      
+    @Name("concurrentFactoryTest.dependentFactory")
+    static public class DependentFactory {
+        @Factory(value = "concurrentFactoryTest.dependentString", scope = APPLICATION, autoCreate = true)
+        public String createString() {
+           return (String) Expressions.instance().createValueExpression("#{concurrentFactoryTest.LockHoldingComponent.string}").getValue();
+        }
+    }
     
-    @Name("concurrentFactoryTest.SlowFactory")
-    static public class SlowFactory {
-        @Factory(value = "concurrentFactoryTest.slowlyCreatedString", scope = APPLICATION, autoCreate = true)
-        public String slowlyCreateString() {
-            try
-            {
-               Thread.sleep(1000);
-               return "slowly created String";
-            }
-            catch (InterruptedException e)
-            {
-               e.printStackTrace();
-               return null;
-            }
-        }        
+    @Name("concurrentFactoryTest.plainFactory")
+    static public class PlainFactory {
+        @Factory(value = "concurrentFactoryTest.plainFactoryGeneratedString", scope = APPLICATION, autoCreate = true)
+        public String createPlainString() {
+           return "TestString";
+        }
     }
     
-
-
+    @AfterClass
+    @Override
+    public void end()
+    {
+       // don't attempt to endSession, as it will block in the deadlocked org.jboss.seam.Component.getInstanceFromFactory lock
+    }
 }

Added: branches/community/Seam_2_2/src/test/integration/src/org/jboss/seam/test/integration/FactoryLockTest.java
===================================================================
--- branches/community/Seam_2_2/src/test/integration/src/org/jboss/seam/test/integration/FactoryLockTest.java	                        (rev 0)
+++ branches/community/Seam_2_2/src/test/integration/src/org/jboss/seam/test/integration/FactoryLockTest.java	2012-07-03 08:57:58 UTC (rev 14973)
@@ -0,0 +1,178 @@
+package org.jboss.seam.test.integration;
+
+import javax.ejb.Local;
+import javax.ejb.Remove;
+import javax.ejb.Stateful;
+
+import org.jboss.seam.Component;
+import org.jboss.seam.ScopeType;
+import org.jboss.seam.annotations.Factory;
+import org.jboss.seam.annotations.JndiName;
+import org.jboss.seam.annotations.Name;
+import org.jboss.seam.annotations.Scope;
+import org.jboss.seam.mock.SeamTest;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+public class FactoryLockTest extends SeamTest
+{
+   private volatile boolean exceptionOccured = false;   
+   
+   private abstract class TestThread extends Thread {
+      public abstract void runTest() throws Exception;
+      
+      @Override
+      public void run()
+      {
+         try
+         {
+            runTest();
+         }
+         catch (Throwable e)
+         {
+            e.printStackTrace();
+            FactoryLockTest.this.exceptionOccured = true;
+         }
+      }
+   }
+   
+   // JBSEAM-4993
+   // The test starts two threads, one evaluates #{factoryLock.test.testOtherFactory()} and the other #{factoryLock.testString} 200ms later
+   @Test
+   public void factoryLock() 
+       throws Exception 
+   {
+      exceptionOccured = false;
+      Thread thread1 = new TestThread() {
+         @Override
+         public void runTest() throws Exception
+         {
+            FactoryLockTest.this.invokeMethod("foo", "#{factoryLock.test.testOtherFactory()}");
+         }
+      };
+
+      Thread thread2 = new TestThread() {
+         @Override
+         public void runTest() throws Exception
+         {
+            Thread.sleep(200);
+            FactoryLockTest.this.getValue("testString", "#{factoryLock.testString}");
+         }
+      };
+
+      thread1.start();
+      thread2.start();
+   
+      thread1.join();
+      thread2.join();
+      
+      assert !exceptionOccured;
+   }
+   
+   // This test is the same as factoryLock test, except it uses the same factory in both threads.
+   @Test
+   public void sameFactoryLock() 
+       throws Exception 
+   {
+      exceptionOccured = false;
+      Thread thread1 = new TestThread() {
+         @Override
+         public void runTest() throws Exception
+         {
+            FactoryLockTest.this.invokeMethod("testString", "#{factoryLock.test.testSameFactory()}");
+         }
+      };
+
+      Thread thread2 = new TestThread() {
+         @Override
+         public void runTest() throws Exception
+         {
+            Thread.sleep(200);
+            FactoryLockTest.this.getValue("testString", "#{factoryLock.testString}");
+         }
+      };
+
+      thread1.start();
+      thread2.start();
+   
+      thread1.join();
+      thread2.join();
+      
+      assert !exceptionOccured;
+   }
+   
+   private void invokeMethod(final String expected, final String el) throws Exception {
+      new ComponentTest() {
+         @Override
+         protected void testComponents() throws Exception {
+            Assert.assertEquals(expected, invokeMethod(el));
+         }
+     }.run();
+   }
+   
+   private void getValue(final String expected, final String el) throws Exception {
+      new ComponentTest() {
+         @Override
+         protected void testComponents() throws Exception {
+            Assert.assertEquals(expected, getValue(el));
+         }
+     }.run();
+   }
+
+   @Local
+   public static interface FactoryLockLocal
+   {
+      public String getTestString();
+      public String testOtherFactory();
+      public String testSameFactory();
+      public void remove();
+   }
+
+   
+   @Stateful
+   @Scope(ScopeType.SESSION)
+   @Name("factoryLock.test")
+   //@JndiName("java:global/test/FactoryLockTest$FactoryLockAction")
+   public static class FactoryLockAction implements FactoryLockLocal
+   {
+      public String testOtherFactory() {
+         try
+         {
+            Thread.sleep(500);
+         }
+         catch (InterruptedException e)
+         {
+            e.printStackTrace();
+         }
+         return (String)Component.getInstance("factoryLock.foo", true);
+      }
+      
+      // gets instance produced by this component's factory 
+      public String testSameFactory() {
+         try
+         {
+            Thread.sleep(500);
+         }
+         catch (InterruptedException e)
+         {
+            e.printStackTrace();
+         }
+         return (String)Component.getInstance("factoryLock.testString", true);
+      }
+      
+      @Factory(value="factoryLock.testString", scope=ScopeType.EVENT)
+      public String getTestString() {
+         return "testString";
+      }
+      @Remove
+      public void remove() {}
+   }
+   
+   @Name("factoryLock.testProducer")
+   public static class TestProducer {
+      @Factory(value="factoryLock.foo", scope=ScopeType.EVENT)
+      public String getFoo() {
+         return "foo";
+      }
+   }
+}

Modified: branches/community/Seam_2_2/src/test/integration/src/org/jboss/seam/test/integration/testng.xml
===================================================================
--- branches/community/Seam_2_2/src/test/integration/src/org/jboss/seam/test/integration/testng.xml	2012-07-02 12:59:07 UTC (rev 14972)
+++ branches/community/Seam_2_2/src/test/integration/src/org/jboss/seam/test/integration/testng.xml	2012-07-03 08:57:58 UTC (rev 14973)
@@ -10,6 +10,7 @@
 			<class name="org.jboss.seam.test.integration.JavaBeanEqualsTest" />
 			<class name="org.jboss.seam.test.integration.NamespaceResolverTest" />
 			<class name="org.jboss.seam.test.integration.ConcurrentFactoryTest" />
+			<class name="org.jboss.seam.test.integration.FactoryLockTest" />
 		</classes>
 	</test>
 	<test name="Seam Integration Tests Persistence">



More information about the seam-commits mailing list