You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2014/11/27 14:28:14 UTC

[Bug 57274] New: Annotation scanning can cause classes to skip class transformation

https://issues.apache.org/bugzilla/show_bug.cgi?id=57274

            Bug ID: 57274
           Summary: Annotation scanning can cause classes to skip class
                    transformation
           Product: Tomcat 8
           Version: 8.0.14
          Hardware: PC
                OS: Mac OS X 10.1
            Status: NEW
          Severity: major
          Priority: P2
         Component: Catalina
          Assignee: dev@tomcat.apache.org
          Reporter: pverheyden@broadleafcommerce.com

Full context, I am using Spring 4.1.2.RELEASE in my application with Spring
Instrument hooked up as a -javaagent
(-javaagent:/path/to/spring-instrument-4.1.2.RELEASE). We are relying on class
transformation to transform some of our JPA classes and add new methods/fields
on startup. However, I observed that for some classes, class transformation is
completely skipped.

After some debugging, I have narrowed this down to the getResourceInternal
method of WebappClassLoaderBase. Specifically, this is giving me grief:

    protected ResourceEntry findResourceInternal(final String name, final
String path) {

        if (!state.isAvailable()) {
            log.info(sm.getString("webappClassLoader.stopped", name));
            return null;
        }

        if (name == null || path == null) {
            return null;
        }
        // This is returning a non-null entry. On every other class that I
observed, this returns null and it continues on to the class transformers
        ResourceEntry entry = resourceEntries.get(path);
        if (entry != null) {
            return entry;
        }
    ...
    ...
    // Remaining implementation excluded, but below here is where it loops
through the configured `ClassFileTransformer`s

When my JPA persistent unit is loaded, about 98% of the classes return null for
resourceEntries.get(path). Some classes return a non-null entry, and so they
are immediately returned thus skipping the class transformation below.

With more debugging, I came across the code on startup that scans every class
in every jar in the web application on startup to look for SCIs
(ContextConfig.webConfig()). For almost every class that it looks for, it grabs
an input stream based for the class file based on the FileInputStream and then
puts it in a cache.

However, this process also looks for super classes (from ContextConfig):

    private void populateJavaClassCache(String className, JavaClass javaClass)
{
        if (javaClassCache.containsKey(className)) {
            return;
        }

        // Add this class to the cache
        javaClassCache.put(className, new JavaClassCacheEntry(javaClass));

        populateJavaClassCache(javaClass.getSuperclassName());

        for (String iterface : javaClass.getInterfaceNames()) {
            populateJavaClassCache(iterface);
        }
    }

where populateJavaClassCache:

    private void populateJavaClassCache(String className) {
        if (!javaClassCache.containsKey(className)) {
            String name = className.replace('.', '/') + ".class";
            try (InputStream is =
context.getLoader().getClassLoader().getResourceAsStream(name)) {
                if (is == null) {
                    return;
                }
                ClassParser parser = new ClassParser(is);
                JavaClass clazz = parser.parse();
                populateJavaClassCache(clazz.getClassName(), clazz);
            } catch (ClassFormatException e) {
                log.debug(sm.getString("contextConfig.invalidSciHandlesTypes",
                        className), e);
            } catch (IOException e) {
                log.debug(sm.getString("contextConfig.invalidSciHandlesTypes",
                        className), e);
            }
        }
    }

Using context.getLoader().getClassLoader().getResourceAsStream() causes the
WebappClassLoader to load the class and store it in the resourceEntries map.
However at this point, the WebappClassLoader does not have any
ClassTransformers registered with it yet, and thus no class transformation
happens.

So, if you have any JPA entity that is a superclass of something else, class
transformation would be skipped.

I am not sure exactly what the right fix is here but I feel like it should be
something that skips using the WebappClassLoader to load the class? That could
be very expensive though as we would need to scan through all of the jars in
the classpath again to find the superclass definition (which would make this
O(n2).

I have some additional information at
https://github.com/BroadleafCommerce/BroadleafCommerce/issues/1171 but I put in
just the Tomcat-relevant info here.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 57274] Annotation scanning can cause classes to skip class transformation

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57274

--- Comment #1 from Phillip <pv...@broadleafcommerce.com> ---
I think that this issue can be closed. The problems that I highlighted below
are a necessary outcome of Tomcat needing to scan for
ServletContainerInitializers in the web application. It does this by looping
through all of the classes in all of the jars loaded by the application and
reading them in with a FileInputStream. I would imagine that it is a
performance improvement to, after loading a class via FileInputStream, to go
ahead and load the superclass and interfaces into the cache as well. However,
the superclass has to be loaded through the WebappClassLoader which is what
causes a problem in my case.

After doing some additional digging (specifically at
http://java.net/jira/browse/SERVLET_SPEC-36), I found that I can disable the
jar scanning completely by adding an empty <absolute-ordering /> in my web.xml.
After testing this everything in my application works as I would expect.

Sorry for the false positive here but hopefully someone else will come across
this research and will save them some debugging time.

Also, I would also like to take this opportunity to say thanks for all the
contributors to Tomcat! We love the simplicity and performance, and we
consistently recommend Tomcat as our #1 servlet container for those that use
Broadleaf. Thanks again!

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 57274] Annotation scanning can cause classes to skip class transformation

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57274

--- Comment #4 from Phillip <pv...@broadleafcommerce.com> ---
The solution that I had was to completely disable the scanning by adding an
empty <absolute-ordering /> tag into my web.xml. This obviously means that
there is no way to have both my Spring/persistent unit class transformation
along with the ServletContainerInitializers (not an issue in my case).

I believe that one possible solution here would be to disable what I believe to
be a performance improvement of loading the classes by the class loader. If the
classes were not loaded by the classloader at all and instead every single
class was loaded by the FileInputStream, then this would not be a problem at
all. Perhaps there could be some sort of startup flag that controls whether or
not to use the class loader to look for ServletContainerInitializers or to skip
it and load all of them by FileInputStreams?

Mark, what do you mean by "delay the transformation until the the class is
declared"? Does this mean moving the transformation code from WebAppClassLoader
to after it has retrieved the cache from resourceEntries, but prior to actually
loading the class itself (like via a Class.forName())? This seems like a better
solution to me. It seems there is a slight deviation by looking up a class by
name with a getResourceAsStream() and referencing it by doing a
Class.forName().

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 57274] Annotation scanning can cause classes to skip class transformation

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57274

--- Comment #2 from Mark Thomas <ma...@apache.org> ---
Happy to leave this closed but I wanted to add the following comments:

1. If the transformation attempts to make a change that would trigger something
in the annotation scanning then that change is going to be ignored. I think I
am OK with that. There are better ways to achieve the desired result.

2. One way of fixing this would be to delay the transformation until the the
class is declared.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 57274] Annotation scanning can cause classes to skip class transformation

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57274

--- Comment #5 from Mark Thomas <ma...@apache.org> ---
(In reply to Phillip from comment #4)

> Mark, what do you mean by "delay the transformation until the the class is
> declared"? Does this mean moving the transformation code from
> WebAppClassLoader to after it has retrieved the cache from resourceEntries,
> but prior to actually loading the class itself (like via a Class.forName())?

Yes. Basically delay the transformation until just before we call
defineClass().

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 57274] Annotation scanning can cause classes to skip class transformation

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57274

--- Comment #3 from Nick Williams <ni...@nicholaswilliams.net> ---
I will think about this some. Off the top of my head, I don't know what the
answer is, but it sure would be nice if there were a solution.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 57274] Annotation scanning can cause classes to skip class transformation

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57274

Phillip <pv...@broadleafcommerce.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |WONTFIX

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org