Hi Alan, hi Sergei, i see two issues with this patch, the first one is that before that change, ClassFileTransformer was a functional interface, it's not a functional interface anymore, so it will break code (note that ClassFileTransformer is not annotated with @FunctionalInterface so it's a kind of 'legal' non backward compatible change) the second one is more problematic, currently, the API allows me to have access to the ClassLoader even if there is a SecurityManager installed, with the new method "transform" I have not access (at least easily*) to the ClassLoader without fearing a potential SecurityException, i think this should be fixed by sending the module *and* the classloader to transform.
Rémi * there is still a nasty way to get the classloader without getting the security exception but i let that to an astute reader. ----- Mail original ----- > De: "serguei spitsyn" <serguei.spit...@oracle.com> > À: "Alan Bateman" <alan.bate...@oracle.com>, "jigsaw-dev" > <jigsaw-dev@openjdk.java.net> > Envoyé: Lundi 29 Février 2016 19:29:28 > Objet: Re: Round #3: RFR: 8147467 - Add ClassFileTransformer transform > method that provide the Module to the agent > > On 2/29/16 04:29, Alan Bateman wrote: > > On 29/02/2016 08:32, serguei.spit...@oracle.com wrote: > >> : > >>> > >>> Please, review the fix for: > >>> https://bugs.openjdk.java.net/browse/JDK-8147467 > >>> > >>> > >>> Jdk webrev: > >>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/jdk/8147467-Jigsaw-agents.jdk3/ > >>> > > This looks good and I can get this into the jake/jdk repo for you. > > Thanks, Alan! > > > > > I think the javadoc will need a few additional edits but I can look > > after these. In particular, the links to the transform method need to > > be updated as it's the new transform method that is invoked. In > > addition I think we can make the class description of > > ClassFileTransformer a bit clearer. > > Sure, some correction of the javadoc is needed to make it clearer. > > > Thanks, > Serguei > > > > > > -Alan. > >
