DO NOT REPLY [Bug 19282] New: - CocoonComponentManager must be loaded in own classloader

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=19282>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=19282

CocoonComponentManager must be loaded in own classloader

           Summary: CocoonComponentManager must be loaded in own classloader
           Product: Cocoon 2
           Version: Current CVS 2.1
          Platform: Other
        OS/Version: Other
            Status: NEW
          Severity: Critical
          Priority: Other
         Component: core
        AssignedTo: cocoon-dev@xml.apache.org
        ReportedBy: leo.sutic@inspireinfrastructure.com


The CocoonComponentManager uses static variables for component management.

This completely obliterates the ability to have several cocoon instances 
per classloader.

As I understand it, the CocoonComponentManager is a Singleton with respect to 
the Cocoon instance, right?

This means that the static variables could in theory be localized to that 
instance, if it weren't for the need to access them via static accessors.

This one is no problem, as it is thread local, and, as I understand it, empty 
at the start of a request and empty at the end of one.

    /** The environment information */
    private static InheritableThreadLocal environmentStack = new 
CloningInheritableThreadLocal();

So that variable should be just fine and we're not going to have any 
classloader issues here.

This one I don't get:

    private static Map sitemapConfigurationHolders = new HashMap(15);

Can these be made thread-local, or per-instance?

            holder = (SitemapConfigurationHolder)sitemapConfigurationHolders.get
( role );
            if ( null == holder ) {
                // create new holder
                holder = new DefaultSitemapConfigurationHolder( role );
                sitemapConfigurationHolders.put( role, holder );
            }

This one, however, is poison:

    private static ComponentManager rootManager;

Can it be made instance-specific? It is only used as a default in:

    public ComponentManager getSitemapComponentManager()

and that method is only referenced from AbstractEnvironment in the
starting() method. Ironically, the Environment passes through the 
CocoonComponentManager itself, so it is a no-brainer to pass the manager in 
right then and there.

However, the SourceUtil and JSCocoon classes also reference this method.

The question is then: Is JSCocoon's invocations done before or after the

Environment is set up? In that case, the getSitemapComponentManager can return 
null if no environment is set up:

    public ComponentManager getSitemapComponentManager() {
        final EnvironmentStack stack = (EnvironmentStack)environmentStack.get();
        
        if ( null != stack && !stack.empty()) {
            Object[] o = (Object[])stack.peek();
            return (ComponentManager)o[2];
        }
        
        // if we don't have an environment yet, just return null
        return null; // WAS: rootManager;
    }

Since, if I understand this correctly, the only time you'd fall through was 
when called from the AbstractEnvironment.starting() method, which was unneeded.

The attached patch removes this problem. However, the cost is that the 
getSitemapComponentManager method will return null when there is no sitemap. 
This, however, seems to be in line with the contract.