I wanted to revive the discussion of JDK-8200559 (
https://bugs.openjdk.org/browse/JDK-8200559) which was continued on this
GitHub ticket (https://github.com/openjdk/jdk/pull/3546).

As outlined before, Java agents are typically about more than code
instrumentation but need to define additional classes. There are two
reasons for this:

1. Agents sometimes need to define helper classes. For example, if an
instrumented method returns an interface that is a callback. If the
instrumentation wants to replace this callback, a new class needs to be
defined first.
2. Agents often need to define infrastructure for communicating with the
agent from instrumented code. For example to communicate back metrics,
tracing data or anything else that the agent is supposed to be adding.
Often, this is done by injecting a class into a class loader that can be
reached from instrumented code, which carries a static field that can carry
the "agent state" in the form of a callback object.

Previously, there has been a proposal for adding an overload with an
optional argument to ClassFileTransformer that allows to inject classes
into the package and class loader of the currently instrumented class. This
works mostly well for (1) but not so well for (2). I have however thought
about (2) some more since and wanted to suggest an alternative solution
that would make the original suggestion to solve this issue work.

The idea would be to also add a method to LambdaMetafactory. The method
would allow registering a custom metafactory dispatcher, which can carry an
agent's state and then be utilized to communicate back to the agent. This
would render the infrastructure question (2) obsolete. As a more concrete
example, this would code out the additional code:

public class LambdaMetafactory {

    private static final ConcurrentMap<String, ExternalMetafactory>
DISPATCHERS = new ConcurrentHashMap<>();

    public static void register(String id, ExternalMetafactory metafactory)
{
        Objects.requireNonNull(id, "id");
        Objects.requireNonNull(metafactory, "metafactory");
        ExternalMetafactory previous = DISPATCHERS.putIfAbsent(id,
metafactory);
        if (previous == null) {
            throw new IllegalArgumentException("External metafactory
already registered with id: " + id);
        }
    }

    public static boolean remove(String id, ExternalMetafactory
metafactory) {
      return DISPATCHERS.remove(id, metafatory);
    }

    public static CallSite externalMetafactory(MethodHandles.Lookup caller,
                                               String interfaceMethodName,
                                               MethodType factoryType,
                                               String id,
                                               Object... args) throws
LambdaConversionException {
        ExternalMetafactory dispatcher = DISPATCHERS.get(id);
        if (dispatcher == null) {
            throw new LambdaConversionException("No external metafactory
registered with id: " + id);
        }
        return dispatcher.externalConstant(caller, interfaceMethodName,
factoryType, args);
    }

    @FunctionalInterface
    public interface ExternalMetafactory {

        CallSite externalConstant(MethodHandles.Lookup caller,
                                  String interfaceMethodName,
                                  MethodType factoryType,
                                  Object... args) throws
LambdaConversionException;
    }
}

Before instrumenting any class, the agent would register a dispatcher, for
example by using a UUID as a key. Then a ClassFileTransformer would apply
instrumentations where the key is added to the class file within the
invokedynamic call site. The instrumented class can then find "its"
metafactory and install a call site that, for example, is bound to an
instance that carries the agent's state.

To some extent this metafactory would also avoid (1). The invokedynamic
bootstrap can now create a new class loader that is a child of the
instrumented class's class loader which contains the auxiliary classes
directly. Often, the created class loader would also search for classes on
the agent's class loader, which results in a very natural programming
experience as instrumentations can access both the agent's state and the
instrumented class's (or framework's) types.

I am already offering convenience for this in most of Byte Buddy's API, and
it is increasingly better adopted. I also find the resulting agents to be
much simpler to maintain and test. For an open source example, I can point
to the ElasticSearch APM agent: https://github.com/elastic/apm-agent-java

The reason this should be in the JDK is that a custom metafactory class
needs to be injected into the boot loader, the only globally visible
loader, and java.base, as the bootstrap callsite needs to be universally
visible. Using the unnamed module and opening all modules to that module
does not feel like a good solution. And there is a long row of strange
class loaders around that only check for java and javax classes when it
comes to delegating down to the boot loader. So in practice it has shown
that a custom package on the boot loader works poorly, unfortunately.

The only limitation of that approach is creating auxiliary classes that
subclass or implement package-private classes or interfaces. But this would
be covered by the original solution that was suggested to solve (1), as the
subclass does not make sense outside of the package anyways.

Summarizing: with a facility like the one above in the java.base module and
a solution as originally suggested by Mandy, I do not see a scenario that
cannot be covered and I think agents could supply everything they do today
also without using Unsafe API.

This would allow for instrumenting all class files compiled to Java 7 or
higher, which is most class files minus some JDBC drivers.Often, these
class files can be upgraded dynamically. For older JDKs, the call site
could be emulated by injecting a class using Unsafe. But for the JDK that
introduced both APIs, using an internal API would no longer be needed.

What do you think about the suggestion?
Thanks, Rafael

abc

Reply via email to