ClassFileTransformer does not apply to anonymous classes

[Remi Forax]

I agree with Vladimir,

You should not be able to transform/redefine a VM anonymous class because the transformer will certainly mess up with the order of the constant pool entries.

Slightly off-topic, about ASM, when you create a ClassWriter [1], you can pass a ClassReader of an existing class, in that case ASM copy the constant pool from the class reader to the class writer so the constant pool is preserved.

Rémi

[1] http://asm.ow2.org/asm50/javadoc/user/org/objectweb/asm/ClassWriter.html#ClassWriter%28org.objectweb.asm.ClassReader,%20int%29

----- Mail original -----
> De: "Vladimir Ivanov" <vladimir.x.ivanov at oracle.com>
> À: "Rafael Winterhalter" <rafael.wth at gmail.com>
> Cc: "Coleen Phillimore" <coleen.phillimore at oracle.com>, core-libs-dev at openjdk.java.net, "serguei.spitsyn at oracle.com
> Spitsyn" <serguei.spitsyn at oracle.com>, "Daniel Daugherty" <daniel.daugherty at oracle.com>
> Envoyé: Vendredi 22 Janvier 2016 18:47:31
> Objet: Re: ClassFileTransformer does not apply to anonymous classes
> 
> Rafael,
> 
> First of all, I'd like to agree on the terminology. There's some
> confusion there. Anonymous term is ambiguous in Java. There are
> anonymous classes on language level and there's
> Unsafe.defineAnonymousClass() which produce anonymous-in-VM-sense
> classes. I prefer to call them VM anonymous classes to avoid confusion.
> 
> I assume that whenever you use anonymous you assume "VM anonymous".
> 
> > thank you for your response. While I completely understand your view
> > from a VM implementor's point of view, as a practicioner I would
> > recommend against it. Not being able to instrument lambda expressions
> > puts a severe limitation onto using the instrumentation API
> > alltogether. For example, a monitoring application that promises to
> > record all invocations of a method of a certain interface that only
> > works 95% of the time is not 5% less usefull but might no longer be
> > useful at all. People have build large applications based on the
> > assumption that all user application code can be instrumented and such
> > a regression would hurt them. For example, until today, over 30 people
> > that use my code generation framework reached out to me and reported
> > the reported behavior as a bug in my library and people are only
> > starting to migrate their applications to Java 8. The outcome is
> > simply not intuitive as using a lambda expression over an anonyous
> > inner class should not change an application's behavior.
> Can you elaborate on that point, please?
> 
> I don't see any problems with instrumenting user code. Javac represents
> lambda expression body as a private static method of the enclosing
> class, which can be instrumented. VM anonymous classes are used only as
> a mechanism to glue functional interfaces and lambda expressions
> together at runtime.
> 
> For example:
>     static void f(Runnable r) { r.run(); }
>     f(() -> {});
> 
> Test::f (7 bytes)
>    @ 1   Test$$Lambda$1/791452441::run (4 bytes)   inline (hot)
>      @ 0   Test::lambda$main$0 (1 bytes)   inline (hot)
> 
> Why do you need to instrument Test$$Lambda$1/... and not
> Test::lambda$main$0?
> 
> > The currently used workaround that people use is to instrument the
> > LambdaMetaFactory class itself to apply the transformer manually when
> > it is being created. This solution is spreading as a standard approach
> > and I am sure that forbidding a redefinition alltogether would only
> > inspire to other workarrounds for being able to migrate running code
> > to the next Java version.
> It doesn't sound like a workaround, but as a solution for a different
> problem.
> 
> If there's a need to gather profile for every lambda expression
> instantiation site (indy call), then redefining bootstrap method is the
> right way to go IMO. It allows to intercept and customize indy call site
> binding.
> 
> What does sound as a workaround is an attempt to instrument classes
> produced by LambdaMetaFactory. Right now, it generates a fresh VM
> anonymous class on every request, but it is an implementation detail and
> not a requirement.
> 
> For example, LambdaMetaFactory can reuse wrappers on per-functional
> interface basis. It would provide significant footprint savings for
> lambda expression-rich code bases (usually, there are much more
> instantiations than functional interface flavors).
> 
> > Furthermore, I do not think that not being able to patch constant pool
> > indices in the generated code introduces any problems in practice.
> > Most real-world instrumentation only appends new constant pool entries
> > without interfering with the existant pool. Rather, I would like to
> > see an exception being raised if one attempts to change the original
> > constant pool without me being able to consider this from a technical
> > perspective. I think this would serve to be sufficient for most
> > people.
> If you care only about lambda expressions, then constant pool patching
> shouldn't be a problem (AFAIR it isn't used for lambda expressions). But
> in a more generic case (even for java.lang.invoke purposes), the
> coupling between class file and CP patches is very tight and can be
> easily broken with benign class file transformations. Considering ASM
> library, I'm not sure that even simple instrumentations preserve
> original constant pool structure.
> 
> Probably, VM can do some structural checks on CP to forbid any
> modifications except appends, but there are still aliasing problems -
> sharing doesn't work (even for string constants), so new code shouldn't
> use original CP entries.
> 
> I'd prefer to avoid such increase in complexity in VM implementation.
> 
> > I really hope that there is a way to allow for patching anonymously
> > loaded classes even without exposing the constant pool patches.
>  From VM implementation perspective I don't see any problems with
> allowing class file redefinition for VM anonymous classes. What I want
> to do is to discourage from doing so. Again - it is very fragile and
> there's little we can do to fix that.
> 
> > So far, thank you for looking into this. I am sure this a more complex
> > matter than what I am able to comprehend. I appreciate that you are
> > taking the time to consider my opinion!
> 
> Best regards,
> Vladimir Ivanov
>