Author: hlship
Date: Tue Mar 18 19:24:33 2008
New Revision: 638683

URL: http://svn.apache.org/viewvc?rev=638683&view=rev
Log:
TAPESTRY-2267: Services that are dependencies of other EagerLoad services may 
not be eagerly loaded

Modified:
    
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/Module.java
    
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java
    
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/RegistryImpl.java

Modified: 
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/Module.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/Module.java?rev=638683&r1=638682&r2=638683&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/Module.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/Module.java
 Tue Mar 18 19:24:33 2008
@@ -1,4 +1,4 @@
-// Copyright 2006, 2007 The Apache Software Foundation
+// Copyright 2006, 2007, 2008 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -25,9 +25,8 @@
 import java.util.Set;
 
 /**
- * A module within the Tapestry IoC registry. Each Module is constructed 
around a corresponding
- * module builder instance; the methods and annotations of that instance 
define the services
- * provided by the module.
+ * A module within the Tapestry IoC registry. Each Module is constructed 
around a corresponding module builder instance;
+ * the methods and annotations of that instance define the services provided 
by the module.
  */
 public interface Module extends ModuleBuilderSource
 {
@@ -43,9 +42,8 @@
     <T> T getService(String serviceId, Class<T> serviceInterface);
 
     /**
-     * Locates the ids of all services that implement the provided service 
interface, or whose
-     * service interface is assignable to the provided service interface (is a 
super-class or
-     * super-interface).
+     * Locates the ids of all services that implement the provided service 
interface, or whose service interface is
+     * assignable to the provided service interface (is a super-class or 
super-interface).
      *
      * @param serviceInterface the interface to search for
      * @return a collection of service ids
@@ -53,11 +51,10 @@
     Collection<String> findServiceIdsForInterface(Class serviceInterface);
 
     /**
-     * Locates all the decorators that should apply the identified service. 
This includes visibility
-     * rules (private services may only be decorated by decorators in the same 
module) and other
-     * filtering rules. The resulting list is ordered and from the list of
-     * [EMAIL PROTECTED] org.apache.tapestry.ioc.def.DecoratorDef}s, a list of 
[EMAIL PROTECTED] ServiceDecorator}s is
-     * returned.
+     * Locates all the decorators that should apply the identified service. 
This includes visibility rules (private
+     * services may only be decorated by decorators in the same module) and 
other filtering rules. The resulting list is
+     * ordered and from the list of [EMAIL PROTECTED] 
org.apache.tapestry.ioc.def.DecoratorDef}s, a list of [EMAIL PROTECTED]
+     * ServiceDecorator}s is returned.
      *
      * @param serviceId identifies the service to be decorated
      * @return the ordered list of service decorators
@@ -65,8 +62,8 @@
     List<ServiceDecorator> findDecoratorsForService(String serviceId);
 
     /**
-     * Iterates over any decorator definitions defined by the module and 
returns those that apply to
-     * the provided service definition.
+     * Iterates over any decorator definitions defined by the module and 
returns those that apply to the provided
+     * service definition.
      *
      * @param serviceDef for which decorators are being assembled
      * @return set of decorators, possibly empty (but not null)
@@ -79,10 +76,12 @@
     Set<ContributionDef> getContributorDefsForService(String serviceId);
 
     /**
-     * Locates services with the [EMAIL PROTECTED] 
org.apache.tapestry.ioc.annotations.EagerLoad} annotation
-     * and forces them to instantiate fully. This is part of the Registry 
startup.
+     * Locates services with the [EMAIL PROTECTED] 
org.apache.tapestry.ioc.annotations.EagerLoad} annotation and generates proxies
+     * for them, then adds them to the proxies list for instantiation.
+     *
+     * @param proxies collection of proxies to which any eager load services 
in the module should be added
      */
-    void eagerLoadServices();
+    void collectEagerLoadServices(Collection<EagerLoadServiceProxy> proxies);
 
     /**
      * Returns the service definition for the given service id.
@@ -93,8 +92,8 @@
     ServiceDef getServiceDef(String serviceId);
 
     /**
-     * Returns the name used to obtain a logger for the module. Services 
within the module suffix
-     * this with a period and the service id.
+     * Returns the name used to obtain a logger for the module. Services 
within the module suffix this with a period and
+     * the service id.
      *
      * @return module logger name
      */

Modified: 
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java?rev=638683&r1=638682&r2=638683&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java
 Tue Mar 18 19:24:33 2008
@@ -1,4 +1,4 @@
-// Copyright 2006, 2007 The Apache Software Foundation
+// Copyright 2006, 2007, 2008 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -51,9 +51,9 @@
     private Object _moduleBuilder;
 
     /**
-     * A single mutex, shared by all modules, that serializes creation of 
services across all
-     * threads. This is a bit draconian, but appears to be necessary. 
Fortunately, service creation
-     * is a very tiny part of any individual service's lifecycle.
+     * A single mutex, shared by all modules, that serializes creation of 
services across all threads. This is a bit
+     * draconian, but appears to be necessary. Fortunately, service creation 
is a very tiny part of any individual
+     * service's lifecycle.
      */
     private static final Object MUTEX = new Object();
 
@@ -150,7 +150,7 @@
      * @param eagerLoadProxies TODO
      * @return the service proxy
      */
-    private Object findOrCreate(ServiceDef def, List<EagerLoadServiceProxy> 
eagerLoadProxies)
+    private Object findOrCreate(ServiceDef def, 
Collection<EagerLoadServiceProxy> eagerLoadProxies)
     {
         synchronized (MUTEX)
         {
@@ -168,10 +168,8 @@
         }
     }
 
-    public void eagerLoadServices()
+    public void collectEagerLoadServices(Collection<EagerLoadServiceProxy> 
proxies)
     {
-        List<EagerLoadServiceProxy> proxies = newList();
-
         synchronized (MUTEX)
         {
             for (String serviceId : _moduleDef.getServiceIds())
@@ -180,19 +178,15 @@
 
                 if (def.isEagerLoad()) findOrCreate(def, proxies);
             }
-
-            for (EagerLoadServiceProxy proxy : proxies)
-                proxy.eagerLoadService();
         }
     }
 
     /**
-     * Creates the service and updates the cache of created services. Access 
is synchronized via
-     * [EMAIL PROTECTED] #MUTEX}.
+     * Creates the service and updates the cache of created services. Access 
is synchronized via [EMAIL PROTECTED] #MUTEX}.
      *
      * @param eagerLoadProxies a list into which any eager loaded proxies 
should be added
      */
-    private Object create(ServiceDef def, List<EagerLoadServiceProxy> 
eagerLoadProxies)
+    private Object create(ServiceDef def, Collection<EagerLoadServiceProxy> 
eagerLoadProxies)
     {
         String serviceId = def.getServiceId();
 
@@ -351,7 +345,7 @@
         classFab.addField("_creator", Modifier.PRIVATE | Modifier.FINAL, 
ObjectCreator.class);
         classFab.addField("_token", Modifier.PRIVATE | Modifier.FINAL, 
ServiceProxyToken.class);
 
-        classFab.addConstructor(new Class[]{ObjectCreator.class, 
ServiceProxyToken.class}, null,
+        classFab.addConstructor(new Class[] { ObjectCreator.class, 
ServiceProxyToken.class }, null,
                                 "{ _creator = $1; _token = $2; }");
 
         // Make proxies serializable by writing the token to the stream.
@@ -361,7 +355,7 @@
         // This is the "magic" signature that allows an object to substitute 
some other
         // object for itself.
         MethodSignature writeReplaceSig = new MethodSignature(Object.class, 
"writeReplace", null,
-                                                              new 
Class[]{ObjectStreamException.class});
+                                                              new Class[] { 
ObjectStreamException.class });
 
         classFab.addMethod(Modifier.PRIVATE, writeReplaceSig, "return 
_token;");
 

Modified: 
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/RegistryImpl.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/RegistryImpl.java?rev=638683&r1=638682&r2=638683&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/RegistryImpl.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/RegistryImpl.java
 Tue Mar 18 19:24:33 2008
@@ -55,8 +55,8 @@
     }
 
     /**
-     * Used to obtain the [EMAIL PROTECTED] 
org.apache.tapestry.ioc.services.ClassFactory} service, which is
-     * crucial when creating runtime classes for proxies and the like.
+     * Used to obtain the [EMAIL PROTECTED] 
org.apache.tapestry.ioc.services.ClassFactory} service, which is crucial when 
creating
+     * runtime classes for proxies and the like.
      */
     static final String CLASS_FACTORY_SERVICE_ID = "ClassFactory";
 
@@ -191,17 +191,23 @@
     }
 
     /**
-     * It's not unreasonable for an eagerly-loaded service to decide to start 
a thread, at which
-     * point we raise issues about improper publishing of the Registry 
instance from the
-     * RegistryImpl constructor. Moving eager loading of services out to its 
own method should
-     * ensure thread safety.
+     * It's not unreasonable for an eagerly-loaded service to decide to start 
a thread, at which point we raise issues
+     * about improper publishing of the Registry instance from the 
RegistryImpl constructor. Moving eager loading of
+     * services out to its own method should ensure thread safety.
      */
     public void performRegistryStartup()
     {
         _eagerLoadLock.lock();
 
+        List<EagerLoadServiceProxy> proxies = CollectionFactory.newList();
+
         for (Module m : _modules)
-            m.eagerLoadServices();
+            m.collectEagerLoadServices(proxies);
+
+        // TAPESTRY-2267: Gather up all the proxies before instantiating any 
of them.
+
+        for (EagerLoadServiceProxy proxy : proxies)
+            proxy.eagerLoadService();
 
         getService("RegistryStartup", Runnable.class).run();
 


Reply via email to