[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

Mark Thomas  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|REOPENED|RESOLVED

--- Comment #15 from Mark Thomas  ---
This was fixed in r1823460

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

--- Comment #14 from Rainer Jung  ---
Finally for the sake of completeness the stacks of the two loading calls for
the class to get woven.

First call (happening before Spring called addTransformer()). Here
name="de/kippdata/demo/weave/Greeting.class",
path="/de/kippdata/demo/weave/Greeting.class", so the class was loaded as a
resource:

at
org.apache.catalina.loader.WebappClassLoaderBase.findResourceInternal(WebappClassLoaderBase.java:3194)
at
org.apache.catalina.loader.WebappClassLoaderBase.findResource(WebappClassLoaderBase.java:1473)
at
org.apache.catalina.loader.WebappClassLoaderBase.getResourceAsStream(WebappClassLoaderBase.java:1701)
at
org.springframework.core.OverridingClassLoader.openStreamForClass(OverridingClassLoader.java:166)
at
org.springframework.core.OverridingClassLoader.loadBytesForClass(OverridingClassLoader.java:142)
at
org.springframework.context.support.ContextTypeMatchClassLoader$ContextOverridingClassLoader.loadClassForOverriding(ContextTypeMatchClassLoader.java:111)
at
org.springframework.core.OverridingClassLoader.loadClass(OverridingClassLoader.java:90)
at java.lang.ClassLoader.loadClass(ClassLoader.java:247)
at
org.springframework.core.OverridingClassLoader.loadClass(OverridingClassLoader.java:84)
at
org.springframework.context.support.ContextTypeMatchClassLoader.loadClass(ContextTypeMatchClassLoader.java:72)
at org.springframework.util.ClassUtils.forName(ClassUtils.java:250)
at
org.springframework.beans.factory.support.AbstractBeanFactory.doResolveBeanClass(AbstractBeanFactory.java:1429)
at
org.springframework.beans.factory.support.AbstractBeanFactory.resolveBeanClass(AbstractBeanFactory.java:1377)
at
org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.determineTargetType(AbstractAutowireCapableBeanFactory.java:641)
at
org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.predictBeanType(AbstractAutowireCapableBeanFactory.java:609)
at
org.springframework.beans.factory.support.AbstractBeanFactory.isFactoryBean(AbstractBeanFactory.java:1484)
at
org.springframework.beans.factory.support.DefaultListableBeanFactory.doGetBeanNamesForType(DefaultListableBeanFactory.java:425)
at
org.springframework.beans.factory.support.DefaultListableBeanFactory.getBeanNamesForType(DefaultListableBeanFactory.java:395)
at
org.springframework.context.support.PostProcessorRegistrationDelegate.invokeBeanFactoryPostProcessors(PostProcessorRegistrationDelegate.java:81)
at
org.springframework.context.support.AbstractApplicationContext.invokeBeanFactoryPostProcessors(AbstractApplicationContext.java:687)
at
org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:525)
at
org.springframework.web.context.ContextLoader.configureAndRefreshWebApplicationContext(ContextLoader.java:443)
at
org.springframework.web.context.ContextLoader.initWebApplicationContext(ContextLoader.java:325)
at
org.springframework.web.context.ContextLoaderListener.contextInitialized(ContextLoaderListener.java:107)
at
org.apache.catalina.core.StandardContext.listenerStart(StandardContext.java:5109)
at
org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5632)
at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:145)
at
org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:1015)
at
org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:991)
at
org.apache.catalina.core.StandardHost.addChild(StandardHost.java:652)
at
org.apache.catalina.startup.HostConfig.deployWAR(HostConfig.java:1127)
at
org.apache.catalina.startup.HostConfig$DeployWar.run(HostConfig.java:2020)
at
java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:439)
at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303)
at java.util.concurrent.FutureTask.run(FutureTask.java:138)
at
java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:895)
at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:918)
at java.lang.Thread.run(Thread.java:662)


Second call, now with transformers in place but with class already cached. Here
name="de.kippdata.demo.weave.Greeting",
path="/de/kippdata/demo/weave/Greeting.class", so the class was loaded as an
actual class:

at
org.apache.catalina.loader.WebappClassLoaderBase.findResourceInternal(WebappClassLoaderBase.java:3194)
at
org.apache.catalina.loader.WebappClassLoaderBase.findClassInternal(WebappClassLoaderBase.java:3060)
at
org.apache.catalina.loader.WebappClassLoaderBase.findClass(WebappClassLoaderBase.java:1388)
 

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

--- Comment #13 from Rainer Jung  ---
The example webapp is available at

https://gist.github.com/rainerjung/99802d9cf321b9c594665b2933d8ea49

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

--- Comment #12 from Rainer Jung  ---
The fix for trunk, which was backported to 8.5 and 8 but not to seven was:

r1730946 | markt | 2016-02-17 22:16:40 +0100 (Wed, 17 Feb 2016) | 1 line

Refactor class loading so JAR scanning does not trigger the caching of the
byte[] for every scanned class until the class is loaded.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

--- Comment #11 from Rainer Jung  ---
Created attachment 35700
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=35700=edit
Suggested patch for TC 7

Minimal patch for TC 7

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

Rainer Jung  changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|FIXED   |---

--- Comment #10 from Rainer Jung  ---
Reopening this for TC 7.

My situation:

- not using Springs TomcatInstrumentableClassLoader  but instead relying on the
normal Tomcat WebappClassLoader (and WebappClassLoaderBase) implementing
InstrumentableClassLoader

- using Spring load time weaving

That combination works starting with the implementation of
InstrumentableClassLoader in 7.0.64 but is broken again since 7.0.70.

Reason is the reorganization of the resourceEntries cache. During context
initialization the class which should get woven is loaded two times. Once as a
resource (and at a time when the weaver was not yet added to the class loader)
and then again as a class with the weaver in place. Starting with 7.0.70 the
loading as a resource and as a class use the same key in the resourceEntries
cache. Since the weaving does not happen when a class is served from the cache,
this breaks it.

In TC 8, 8.5 and 9 there was a later change in WebappClassLoaderBase which
makes it work again. The weaving was moved from close to the place were
resources get added to resourceEntries in findResourceInternal() to
findClassInternal().

That change also does quite a few other things, so I isolated it moving the
weaver call and tested it in TC 7 with the unit tests. This did not show any
problems.

Originally the problem was observed using Spring 3.0, but the behavior is
unchanged for at least of Spring 3.0-4.3.9.

I will attach a suggested patch and also a simple example webapp named "weave".
Calling the URI /weave/ will respond with "Hello World!" if weaving succeeds,
and with "Unwoven" if not. See the trivial class Greeting, the beans file
weave.xml, its declaration in web.xml and the index.jsp, all of which are very
small. Unfortunately the war file is 6 MB due to the size of the included
Spring jar files.

Regards,

Rainer

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

Violeta Georgieva  changed:

   What|Removed |Added

 Status|REOPENED|RESOLVED
 Resolution|--- |FIXED

--- Comment #9 from Violeta Georgieva  ---
Hi,

This is fixed for 8.0.33 onwards.

Regards,
Violeta

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

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

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

--- Comment #8 from Andrei Ivanov  ---
I think this can be closed now that since 8.0.33 the class cache is gone,
rendering TomcatInstrumentableClassLoader useless, but making the
TomcatLoadTimeWeaver work.

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

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

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

Ahmed Hosni  changed:

   What|Removed |Added

 CC||ahmedhosni...@gmail.com

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

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

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

Mark Thomas  changed:

   What|Removed |Added

 CC||marek.stanulew...@gmail.com

--- Comment #7 from Mark Thomas  ---
*** Bug 58560 has been marked as a duplicate of this bug. ***

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

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

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

Bryn  changed:

   What|Removed |Added

   Severity|normal  |enhancement
 Resolution|WONTFIX |---
 Status|RESOLVED|REOPENED
   Priority|P2  |P3

--- Comment #6 from Bryn  ---
As suggested by Mark I have reopened this as an enhancement.

I have also come across this issue. In short registering a transformer in a
ServletContextInitializer was too late for classes that are scanned as part of
the @HandlesTypes mechanism, they are already loaded and are therefore not
transformed.

It would be great if we could have an alternative mechanism to register
transformers before scanning takes place.

Ideally this would be automatic registration rather than requiring the
developer to configure anything, thus allowing them to include the library with
the transformer in and benefit without referring to installation instructions.
This was my original goal of registering the transformer in a
ServletContextInitializer.

With this in mind the ServiceLoader mechanism or something similar could be
considered.

Library developers would create a resource containing the fully qualified class
names of their transformers at:
META-INF/services/java.lang.instrument.ClassFileTransformer

Transformers would be enumerated and registered before scanning takes place.

Even if JDK ServiceLoader implementation is not used, perhaps due to
performance, then the mechanism is at least well understood by developers.
Instead of having custom config specific to Tomcat the docs can just say that
ClassFileTransformers are discovered via ServiceLoader. This raises the chances
that other servlet containers would adopt the same approach. 

I note that Jetty have an addTransformer method on their classloader as well,
but using ServiceLoader would be container independent.

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

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

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

--- Comment #5 from Andrei Ivanov  ---
Thanks :-)
I've managed to activate the TomcatInstrumentableClassLoader, as the
context.xml in the WAR was ignored when deploying the app in a virtual host
with the context defined there.

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

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

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

--- Comment #4 from Mark Thomas  ---
I had some discussions with the Spring folks at $dayjob and they haven't been
able to come up with a better solution either.

The only other option that did come up in the discussion was avoiding the
annotation and HandlesType scanning. It should be possible to avoid the
annotation scanning by explicit definitions in web.xml. The HandlesType
scanning can be disabled but there is no alternative available to replace it.

You may ned up having to use TomcatInstrumentableClassLoader.

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

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

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

Mark Thomas ma...@apache.org changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |WONTFIX

--- Comment #3 from Mark Thomas ma...@apache.org ---
I don't think we can fix this.

The TomcatLoadTimeWeaver could be configured slightly earlier (in an SCI) but
that is the earliest specification defined hook that could be used. This still
isn't early enough to catch everything since any classes defined in a
HandlesType for an SCI will already have been loaded by this point.

Anything earlier requires Tomcat specific configuration at which point you
might as well use the TomcatInstrumentableClassLoader and configure it in a
META-INF/context.xml.

I'll bring this to the attention of the Spring folks via $dayjob to see if they
can come up with anything better.

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

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

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

--- Comment #2 from Violeta Georgieva violet...@apache.org ---
Hi,

Here are my comments:

The most obvious solution here is to clear
WebappClassLoaderBase.resourceEntries when a transformer is added but I have
some concerns about it.

When you are using TomcatInstrumentableClassLoader it is defined in context.xml
and this class loader will be used to load the classes from the very beginning
(please correct me if this is not the case from spring point of view)

When you are using TomcatLoadTimeWeaver (this bug) the transformer is added on
contextInitialed phase which, my opinion, is too late.
I think that this (TomcatLoadTimeWeaver) should be added on an earlier stage
(e.g. the loader created event or configure_start event). Otherwise:
- ServletContainerInitializers will work with non-transformed classes
- Annotations scanning will happen on non-transformed classes
- Depending on ServletContextListeners order, the listeners before Spring's one
will work with non-transformed classes

Why I think that clearing resourceEntries is not a good solution:
- There might appear ClassCastExceptions between components using
non-transformed and transformed classes.

Other solution will be to mark the loader as modified when a transformer is
added so that the context will be reloaded (if it supports reloadable).
This won't solve the Spring's case because they will add the transformer again
on the contextInitialized step and we will enter in never ending cycle.

Other opinions?

Regards,
Violeta

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

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

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

donnchadh donnch...@gmail.com changed:

   What|Removed |Added

 CC||donnch...@gmail.com

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

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

[Bug 58143] The WebppClassLoader doesn't call transformers on cached classes

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=58143

Andrei Ivanov andrei.iva...@gmail.com changed:

   What|Removed |Added

 OS||All

--- Comment #1 from Andrei Ivanov andrei.iva...@gmail.com ---
This started from https://jira.spring.io/browse/SPR-13210

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

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