[Bug 55317] Facilitate weaving by allowing ClassFileTransformer to be added to WebppClassLoader

[bu...@apache.org]

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

Nick Williams <ni...@nicholaswilliams.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #30749|0                           |1
        is obsolete|                            |

--- Comment #17 from Nick Williams <ni...@nicholaswilliams.net> ---
Created attachment 30825
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=30825&action=edit
Proposed implementation of this feature

(In reply to Mark Thomas from comment #16)
> 1. Why loop over list rather than using contains() in addTransformer() ?
>  
> 3. Why not use List.remove(Object) in removeTransformer() ?

That was my own mistake. I didn't read the Javadoc properly. I have corrected
this in the attached revised patch.

> 4. I'm concerned that synchronizing on the list of transformers while classes are transformed will become a bottleneck when lots of classes are being loaded and the transformer is relatively slow. A separate ReadWriteLock for the transformer list is probably the way to go but really some testing is required to determine if there is an issue here or not.

Another mistake of mine. This could DEFINITELY be a problem if multiple threads
are loading classes at the same time. A ReadWriteLock is definitely preferable
over synchronization here. I have corrected this in the attached revised patch.

> 5. I'm not a fan of the org.apache.tomcat.unittest package unless the classes concerned are going to be used by multiple tests across multiple packages.

Understood. I have relocated/renamed these two classes in accordance with the
discussion on the mailing list. The changes are in the attached revised patch.

> 2. Should addTransformer() be looking for multiple instances of the same Transformer or multiple instances of the same class of Transformer?

No, this was correct. It could be valid to have multiple instances of the same
transformer class, but not multiple copies of the same instance.

An example use case is an application using JPA with Spring Framework. JPA
abstracts away from the java.lang.instrument.ClassFileTransformer by specifying
a javax.persistence.spi.ClassTransformer. JPA providers add ClassTransformers
to the persistence unit instead of ClassFileTransformers. Applying this
directly would require the ClassLoader to support
javax.persistence.spi.ClassTransformer, which won't work in many cases (such as
Tomcat). To get around this, Spring Framework uses a
org.springframework.orm.jpa.persistenceunit.ClassFileTransformerAdapter to wrap
a ClassTransformer with a ClassFileTransformer. If a provider adds multiple
ClassTransformer implementations to the persistence unit, Spring will in turn
add multiple ClassFileTransformerAdapter instances to the ClassLoader. All of
these instances will do something different, but they will be of the same class
as far as WebappClassLoader can tell.

-- 
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