ByteBuddy doesn't handle diamond problem properly
I have default method defined in SomeSharedInterface and have two additional interfaces Trait1 and Trait2 - both extending SomeSharedInterface - i.e. inheriting default implementation of default method in SomeSharedInterface.
I declare new non-abstract class that implements both Trait1 and Trait2 interfaces and call the method, that has default implementation in SomeSharedInterface. Plain java executes method correctly, ByteBuddy proxy receives NULL in @DefaultCall, probably because it thinks that tracking this call is impossible due to ambiguity. But it's resolvable.
I've tested the problem on the latest version available 1.12.5 - because I noticed somewhere on Twitter that the default method handling should have been corrected in latest versions, but it seems that there are still blind spots.
Thanks for your answer, I appreciate the work invested in ByteBuddy!
The problem is demonstrated by following test:
public class DiamondProblemTest {
@Test
public void demonstrateDiamondProblem() throws InstantiationException, IllegalAccessException {
// this works properly - plain java
assertEquals("whatever", new ExampleJavaClass().getCode());
// this does not
final Class<?> proxy = new ByteBuddy()
.subclass(Object.class)
.implement(Trait1.class, Trait2.class)
// AND TRAP ALL METHODS EXCEPT CONSTRUCTORS AND FINALIZER
.method(
ElementMatchers.noneOf(
ElementMatchers.isFinalizer(), ElementMatchers.isConstructor()
)
)
// AND DELEGATE CALL TO OUR INVOCATION HANDLER STORED IN PRIVATE FIELD OF THE CLASS
.intercept(MethodDelegation.to(ByteBuddyDispatcherInvocationHandler.class))
// NOW CREATE THE BYTE-CODE
.make()
// AND LOAD IT IN CURRENT CLASSLOADER
.load(DiamondProblemTest.class.getClassLoader())
// RETURN
.getLoaded();
// that's still ok
final Object theInstance = proxy.newInstance();
assertNotNull(theInstance);
// here I receive NullPointerException
assertEquals("whatever", ((SomeSharedInterface)theInstance).getCode());
}
public interface Trait1 extends SomeSharedInterface {
}
public interface Trait2 extends SomeSharedInterface {
}
public interface SomeSharedInterface {
default String getCode() {
return "whatever";
}
}
public static class ExampleJavaClass implements Trait1, Trait2 {
}
public static class ByteBuddyDispatcherInvocationHandler {
@RuntimeType
public static Object interceptMethodCall(
@This Object proxy,
@Origin Method method,
@SuperCall(nullIfImpossible = true, serializableProxy = true, fallbackToDefault = false) Callable<Object> superMethod,
@DefaultCall(nullIfImpossible = true, serializableProxy = true) Callable<Object> defaultMethod,
@AllArguments Object[] args
) throws Throwable {
// default method should always retrieve default method reference, should it not?!
if (method.isDefault()) {
return defaultMethod.call();
} else {
return superMethod.call();
}
}
}
}
Well, it's more complicated then that. For example, you could not override getCode from ExampleJavaClass without specifying the path downwards:
public static class ExampleJavaClass implements Trait1, Trait2 {
@Override
public String getCode() {
return super.getCode();
}
}
This is the situation Byte Buddy is put. Instead, it needs to specify if it wants to invoke the super method via Trait1.super.getCode() or Trait2.super.getCode(). Byte Buddy normally chooses this for you if there is only one choice, but here you need to specify the interface via targetType = Trait1.class (or the other).
Ok, I understand. For the time being I'm forced to use something like:
public class DiamondProblemTest {
@Test
public void demonstrateDiamondProblem() throws InstantiationException, IllegalAccessException {
// this works properly - plain java
assertEquals("whatever", new ExampleJavaClass().getCode());
// this does not
final Class<?> proxy = new ByteBuddy()
.subclass(Object.class)
.implement(Trait1.class, Trait2.class)
// AND TRAP ALL METHODS EXCEPT CONSTRUCTORS AND FINALIZER
.method(
ElementMatchers.noneOf(
ElementMatchers.isFinalizer(), ElementMatchers.isConstructor()
)
)
// AND DELEGATE CALL TO OUR INVOCATION HANDLER STORED IN PRIVATE FIELD OF THE CLASS
.intercept(MethodDelegation.to(ByteBuddyDispatcherInvocationHandler.class))
// NOW CREATE THE BYTE-CODE
.make()
// AND LOAD IT IN CURRENT CLASSLOADER
.load(DiamondProblemTest.class.getClassLoader())
// RETURN
.getLoaded();
// that's still ok
final Object theInstance = proxy.newInstance();
assertNotNull(theInstance);
// here I receive NullPointerException
assertEquals("whatever", ((SomeSharedInterface)theInstance).getCode());
}
public interface Trait1 extends SomeSharedInterface {
}
public interface Trait2 extends SomeSharedInterface {
}
public interface SomeSharedInterface {
default String getCode() {
return "whatever";
}
}
public static class ExampleJavaClass implements Trait1, Trait2 {
}
public static class ByteBuddyDispatcherInvocationHandler {
@RuntimeType
public static Object interceptMethodCall(
@This Object proxy,
@Origin Method method,
@SuperCall(nullIfImpossible = true, serializableProxy = true, fallbackToDefault = false) Callable<Object> superMethod,
@DefaultCall(nullIfImpossible = true, serializableProxy = true) Callable<Object> defaultMethod,
@AllArguments Object[] args
) throws Throwable {
// default method should always retrieve default method reference, should it not?!
if (method.isDefault()) {
if (defaultMethod == null) {
return findMethodHandle(method).bindTo(proxy).invokeWithArguments(args);
} else {
return defaultMethod.call();
}
} else {
return superMethod.call();
}
}
}
/**
* Finds method handle for the default method.
* @param method
* @return
*/
static MethodHandle findMethodHandle(Method method) {
try {
final Constructor<Lookup> constructor = Lookup.class.getDeclaredConstructor(Class.class, int.class);
constructor.setAccessible(true);
final Class<?> declaringClass = method.getDeclaringClass();
return constructor
.newInstance(declaringClass, Lookup.PRIVATE)
.unreflectSpecial(method, declaringClass);
} catch (Exception ex) {
throw new IllegalArgumentException("Can't find handle to method " + method.toGenericString() + "!", ex);
}
}
}
Of course MethodHandle is good to be cached somewhere - and it's still reflection code that is probably much slower than if it would be supported by ByteBuddy directly.
(note for other people that might run at the problem)
You can use Origin on a method handle parameter. The handle should have the correct privilege for invocation etc.
I tried it, and when calling:
methodHandle.bindTo(proxy).invokeWithArguments(args)
On the handle I got 'java.lang.StackOverflowError'.
When using:
methodHandle.invokeWithArguments(args)
I got java.lang.invoke.WrongMethodTypeException: cannot convert MethodHandle(SomeSharedInterface)String to ()Object.
But my original workaround works - so no need to worry.
Just to clarify the reason for closing. The issue was closed because "the question was answered" but no changes in the codebase were made or it was closed because you've somehow addressed the diamond problem?
Sorry, my bad. I was clearing out old issues, but I will try to look into this.
I added a new value for @Origin to accept MethodHandles.Lookup to make this always possible but this is how it would work (since Java 9):
public static class ByteBuddyDispatcherInvocationHandler {
@RuntimeType
public static Object interceptMethodCall(
@This Object proxy,
@Origin Method method,
@Origin MethodHandles.Lookup lookup,
@SuperCall(nullIfImpossible = true, serializableProxy = true, fallbackToDefault = false) Callable<Object> superMethod,
@DefaultCall(nullIfImpossible = true, serializableProxy = true) Callable<Object> defaultMethod,
@AllArguments Object[] args
) throws Throwable {
// default method should always retrieve default method reference, should it not?!
if (method.isDefault()) {
if (defaultMethod == null) {
return MethodHandles.privateLookupIn(SomeSharedInterface.class, lookup)
.findSpecial(SomeSharedInterface.class, "getCode", MethodType.methodType(String.class), SomeSharedInterface.class)
.bindTo(proxy)
.invokeWithArguments(args);
} else {
return defaultMethod.call();
}
} else {
return superMethod.call();
}
}
}
Alternatively, you could create your own binding to supply your own proxy. Of course, the resulting method handle should be cached for reuse.
The reason this cannot be resolved is that both Trait1 and Trait2 could later introduce an override of the method which requires to make this explicit. This is in principal not a determinate invocation.
Thank you! What version this change will be part of? I have an unit test for this situation, so I can test the new approach there.
Next release.
I tested it on master but always happy if you validate the solution on your end.
I can confirm, it works with locally built 1.12.23-SNAPSHOT version of ByteBuddy.
The generic code, that derives the called method from the context is:
final Callable<Object> superCallable;
if (method.isDefault()) {
if (defaultMethod == null) {
superCallable = () -> {
try {
return MethodHandles.privateLookupIn(method.getDeclaringClass(), lookup)
.findSpecial(
method.getDeclaringClass(),
method.getName(),
MethodType.methodType(String.class),
method.getDeclaringClass()
)
.bindTo(proxy)
.invokeWithArguments(args);
} catch (Throwable e) {
throw new InvocationTargetException(e);
}
};
} else {
superCallable = defaultMethod;
}
} else {
superCallable = superMethod;
}
The test-case with diamond problem passes in my code. Thank you for your support!