https://issues.apache.org/bugzilla/show_bug.cgi?id=55317
Nick Williams <nicho...@nicholaswilliams.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30749|0 |1 is obsolete| | --- Comment #17 from Nick Williams <nicho...@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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org