From 78f5cf3a955361bb5fd46048e10a442af4a35b01 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Mon, 14 Jun 2021 15:48:42 -0500 Subject: [PATCH 1/2] Use handles to dispath default methods Default methods on interfaces fed to java.lang.Proxy cannot be invoked without using method handles, which requires a Lookup with private access (acquired within the interface class). This commit adds a load path that propagates such a Lookup through to the eventual Proxy instance, allowing it to skip binding default methods to their (nonexistent) native function, instead calling the provided default method body. Fixes #249. --- src/main/java/jnr/ffi/LibraryLoader.java | 26 ++++++++++++++++--- .../java/jnr/ffi/provider/FFIProvider.java | 12 +++++++++ .../jnr/ffi/provider/InvalidProvider.java | 8 +++++- .../ffi/provider/NativeInvocationHandler.java | 19 +++++++++++++- .../ffi/provider/jffi/AsmLibraryLoader.java | 10 +++++-- .../jnr/ffi/provider/jffi/LibraryLoader.java | 3 ++- .../provider/jffi/NativeLibraryLoader.java | 13 +++++++--- .../java/jnr/ffi/provider/jffi/Provider.java | 6 +++++ .../jffi/ReflectionLibraryLoader.java | 6 +++-- 9 files changed, 89 insertions(+), 14 deletions(-) diff --git a/src/main/java/jnr/ffi/LibraryLoader.java b/src/main/java/jnr/ffi/LibraryLoader.java index b2a03605d..81d4f353a 100644 --- a/src/main/java/jnr/ffi/LibraryLoader.java +++ b/src/main/java/jnr/ffi/LibraryLoader.java @@ -34,6 +34,7 @@ import java.io.File; import java.io.FileReader; import java.io.IOException; +import java.lang.invoke.MethodHandles; import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.lang.reflect.Proxy; @@ -75,9 +76,9 @@ public abstract class LibraryLoader { private final TypeMapper.Builder typeMapperBuilder = new TypeMapper.Builder(); private final FunctionMapper.Builder functionMapperBuilder = new FunctionMapper.Builder(); private final Class interfaceClass; + private final MethodHandles.Lookup lookup; private boolean failImmediately = false; - /** * Creates a new {@code LibraryLoader} instance. * @@ -89,8 +90,26 @@ public static LibraryLoader create(Class interfaceClass) { return FFIProvider.getSystemProvider().createLibraryLoader(interfaceClass); } + /** + * Creates a new {@code LibraryLoader} instance. + * + * @param The library type. + * @param interfaceClass the interface that describes the native library functions + * @param lookup the {@link java.lang.invoke.MethodHandles.Lookup} to use for invoking default functions + * @return A {@code LibraryLoader} instance. + */ + public static LibraryLoader create(Class interfaceClass, MethodHandles.Lookup lookup) { + return FFIProvider.getSystemProvider().createLibraryLoader(interfaceClass, lookup); + } + protected LibraryLoader(Class interfaceClass) { this.interfaceClass = interfaceClass; + this.lookup = null; + } + + protected LibraryLoader(Class interfaceClass, MethodHandles.Lookup lookup) { + this.interfaceClass = interfaceClass; + this.lookup = lookup; } /** @@ -415,7 +434,7 @@ public T load() { optionMap.put(LibraryOption.FunctionMapper, functionMappers.size() > 1 ? new CompositeFunctionMapper(functionMappers) : functionMappers.get(0)); try { - return loadLibrary(interfaceClass, Collections.unmodifiableList(libraryNames), getSearchPaths(), + return loadLibrary(interfaceClass, lookup, Collections.unmodifiableList(libraryNames), getSearchPaths(), Collections.unmodifiableMap(optionMap), failImmediately); } catch (LinkageError error) { @@ -453,13 +472,14 @@ private Collection getSearchPaths() { * Implemented by FFI providers to load the actual library. * * @param interfaceClass The java class that describes the functions to be mapped. + * @param lookup The lookup to use for invoking default methods, if necessary * @param libraryNames A list of libraries to load and search for symbols. * @param searchPaths The paths to search for libraries to be loaded. * @param options The options to apply when loading the library. * @param failImmediately whether to fast-fail when the library does not implement the requested functions * @return an instance of {@code interfaceClass} that will call the native methods. */ - protected abstract T loadLibrary(Class interfaceClass, Collection libraryNames, + protected abstract T loadLibrary(Class interfaceClass, MethodHandles.Lookup lookup, Collection libraryNames, Collection searchPaths, Map options, boolean failImmediately); diff --git a/src/main/java/jnr/ffi/provider/FFIProvider.java b/src/main/java/jnr/ffi/provider/FFIProvider.java index a7bb65dc3..f9e43a5f8 100644 --- a/src/main/java/jnr/ffi/provider/FFIProvider.java +++ b/src/main/java/jnr/ffi/provider/FFIProvider.java @@ -20,6 +20,8 @@ import jnr.ffi.LibraryLoader; +import java.lang.invoke.MethodHandles; + /** * This class defines the facilities a JNR FFI provider must provide. * @@ -53,6 +55,16 @@ protected FFIProvider() {} */ public abstract LibraryLoader createLibraryLoader(Class interfaceClass); + /** + * Creates a new {@link LibraryLoader} instance. + * + * @param The library type. + * @param interfaceClass The library interface class. + * @param lookup the {@link java.lang.invoke.MethodHandles.Lookup} to use for invoking default functions + * @return the {@code LibraryLoader} instance. + */ + public abstract LibraryLoader createLibraryLoader(Class interfaceClass, MethodHandles.Lookup lookup); + private static final class SystemProviderSingletonHolder { private static final FFIProvider INSTANCE = getInstance(); diff --git a/src/main/java/jnr/ffi/provider/InvalidProvider.java b/src/main/java/jnr/ffi/provider/InvalidProvider.java index 4e783a355..ab5a12f20 100644 --- a/src/main/java/jnr/ffi/provider/InvalidProvider.java +++ b/src/main/java/jnr/ffi/provider/InvalidProvider.java @@ -22,6 +22,7 @@ import jnr.ffi.LibraryOption; import jnr.ffi.Runtime; +import java.lang.invoke.MethodHandles; import java.util.Collection; import java.util.Map; @@ -45,11 +46,16 @@ public Runtime getRuntime() { public LibraryLoader createLibraryLoader(Class interfaceClass) { return new LibraryLoader(interfaceClass) { @Override - protected T loadLibrary(Class interfaceClass, Collection libraryNames, Collection searchPaths, Map options, boolean failImmediately) { + protected T loadLibrary(Class interfaceClass, MethodHandles.Lookup lookup, Collection libraryNames, Collection searchPaths, Map options, boolean failImmediately) { UnsatisfiedLinkError error = new UnsatisfiedLinkError(message); error.initCause(cause); throw error; } }; } + + @Override + public LibraryLoader createLibraryLoader(Class interfaceClass, MethodHandles.Lookup lookup) { + return createLibraryLoader(interfaceClass); + } } diff --git a/src/main/java/jnr/ffi/provider/NativeInvocationHandler.java b/src/main/java/jnr/ffi/provider/NativeInvocationHandler.java index 6b285c7a9..62c6e87f2 100644 --- a/src/main/java/jnr/ffi/provider/NativeInvocationHandler.java +++ b/src/main/java/jnr/ffi/provider/NativeInvocationHandler.java @@ -18,6 +18,8 @@ package jnr.ffi.provider; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.util.Collections; @@ -30,6 +32,7 @@ public class NativeInvocationHandler implements InvocationHandler { private volatile Map fastLookupTable; private final Map invokerMap; + private final MethodHandles.Lookup lookup; /** * Creates a new InvocationHandler instance. @@ -37,12 +40,26 @@ public class NativeInvocationHandler implements InvocationHandler { * @param invokers A map of method invokers * */ - public NativeInvocationHandler(Map invokers) { + public NativeInvocationHandler(Map invokers, MethodHandles.Lookup lookup) { this.invokerMap = invokers; this.fastLookupTable = Collections.emptyMap(); + this.lookup = lookup; } public Object invoke(Object self, Method method, Object[] argArray) throws Throwable { + // skip default methods + if (method.isDefault()) { + if (lookup == null) { + throw new AbstractMethodError("default methods not supported without Lookup or code generation"); + } + + MethodHandle handle = lookup.unreflectSpecial(method, method.getDeclaringClass()); + Object[] newArgs = new Object[argArray.length + 1]; + System.arraycopy(argArray, 0, newArgs, 1, argArray.length); + newArgs[0] = self; + return handle.invokeWithArguments(newArgs); + } + Invoker invoker = fastLookupTable.get(method); return invoker != null ? invoker.invoke(self, argArray) : lookupAndCacheInvoker(method).invoke(self, argArray); } diff --git a/src/main/java/jnr/ffi/provider/jffi/AsmLibraryLoader.java b/src/main/java/jnr/ffi/provider/jffi/AsmLibraryLoader.java index 7fe0ff6cf..a9e91eb91 100644 --- a/src/main/java/jnr/ffi/provider/jffi/AsmLibraryLoader.java +++ b/src/main/java/jnr/ffi/provider/jffi/AsmLibraryLoader.java @@ -43,6 +43,7 @@ import org.objectweb.asm.ClassWriter; import java.io.PrintWriter; +import java.lang.invoke.MethodHandles; import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.lang.reflect.ParameterizedType; @@ -72,8 +73,13 @@ public class AsmLibraryLoader extends LibraryLoader { private final NativeRuntime runtime = NativeRuntime.getInstance(); @Override - T loadLibrary(NativeLibrary library, Class interfaceClass, Map libraryOptions, - boolean failImmediately /* ignored, asm loader eagerly binds everything */) { + T loadLibrary( + NativeLibrary library, + Class interfaceClass, + MethodHandles.Lookup lookup /* ignored, asm loader avoids binding default methods */, + Map libraryOptions, + boolean failImmediately /* ignored, asm loader eagerly binds everything */) { + AsmClassLoader oldClassLoader = classLoader.get(); // Only create a new class loader if this was not a recursive call (i.e. loading a library as a result of loading another library) diff --git a/src/main/java/jnr/ffi/provider/jffi/LibraryLoader.java b/src/main/java/jnr/ffi/provider/jffi/LibraryLoader.java index ebf840ff0..d6f5e4bde 100644 --- a/src/main/java/jnr/ffi/provider/jffi/LibraryLoader.java +++ b/src/main/java/jnr/ffi/provider/jffi/LibraryLoader.java @@ -27,6 +27,7 @@ import jnr.ffi.mapper.TypeMapper; import jnr.ffi.provider.NullTypeMapper; +import java.lang.invoke.MethodHandles; import java.util.Map; public abstract class LibraryLoader { @@ -60,5 +61,5 @@ static CompositeTypeMapper newClosureTypeMapper(AsmClassLoader classLoader, Sign new CachingTypeMapper(new AnnotationTypeMapper())); } - abstract T loadLibrary(NativeLibrary library, Class interfaceClass, Map libraryOptions, boolean failImmediately); + abstract T loadLibrary(NativeLibrary library, Class interfaceClass, MethodHandles.Lookup lookup, Map libraryOptions, boolean failImmediately); } diff --git a/src/main/java/jnr/ffi/provider/jffi/NativeLibraryLoader.java b/src/main/java/jnr/ffi/provider/jffi/NativeLibraryLoader.java index 4b2cecb59..9d40ad575 100644 --- a/src/main/java/jnr/ffi/provider/jffi/NativeLibraryLoader.java +++ b/src/main/java/jnr/ffi/provider/jffi/NativeLibraryLoader.java @@ -20,6 +20,7 @@ import jnr.ffi.LibraryOption; +import java.lang.invoke.MethodHandles; import java.util.Collection; import java.util.Map; @@ -35,14 +36,18 @@ class NativeLibraryLoader extends jnr.ffi.LibraryLoader { super(interfaceClass); } - public T loadLibrary(Class interfaceClass, Collection libraryNames, Collection searchPaths, - Map options, boolean failImmediately) { + NativeLibraryLoader(Class interfaceClass, MethodHandles.Lookup lookup) { + super(interfaceClass, lookup); + } + + public T loadLibrary(Class interfaceClass, MethodHandles.Lookup lookup, Collection libraryNames, Collection searchPaths, + Map options, boolean failImmediately) { NativeLibrary nativeLibrary = new NativeLibrary(libraryNames, searchPaths); try { return ASM_ENABLED - ? new AsmLibraryLoader().loadLibrary(nativeLibrary, interfaceClass, options, failImmediately) - : new ReflectionLibraryLoader().loadLibrary(nativeLibrary, interfaceClass, options, failImmediately); + ? new AsmLibraryLoader().loadLibrary(nativeLibrary, interfaceClass, lookup, options, failImmediately) + : new ReflectionLibraryLoader().loadLibrary(nativeLibrary, interfaceClass, lookup, options, failImmediately); } catch (RuntimeException ex) { throw ex; diff --git a/src/main/java/jnr/ffi/provider/jffi/Provider.java b/src/main/java/jnr/ffi/provider/jffi/Provider.java index 77cbee6ab..4895c6620 100644 --- a/src/main/java/jnr/ffi/provider/jffi/Provider.java +++ b/src/main/java/jnr/ffi/provider/jffi/Provider.java @@ -21,6 +21,8 @@ import jnr.ffi.Runtime; import jnr.ffi.provider.FFIProvider; +import java.lang.invoke.MethodHandles; + public final class Provider extends FFIProvider { private final NativeRuntime runtime; @@ -36,4 +38,8 @@ public final Runtime getRuntime() { public jnr.ffi.LibraryLoader createLibraryLoader(Class interfaceClass) { return new NativeLibraryLoader(interfaceClass); } + + public jnr.ffi.LibraryLoader createLibraryLoader(Class interfaceClass, MethodHandles.Lookup lookup) { + return new NativeLibraryLoader(interfaceClass, lookup); + } } diff --git a/src/main/java/jnr/ffi/provider/jffi/ReflectionLibraryLoader.java b/src/main/java/jnr/ffi/provider/jffi/ReflectionLibraryLoader.java index 02967ca31..87228c8d9 100644 --- a/src/main/java/jnr/ffi/provider/jffi/ReflectionLibraryLoader.java +++ b/src/main/java/jnr/ffi/provider/jffi/ReflectionLibraryLoader.java @@ -35,6 +35,7 @@ import jnr.ffi.provider.NativeVariable; import java.lang.annotation.Annotation; +import java.lang.invoke.MethodHandles; import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.util.AbstractMap; @@ -51,7 +52,7 @@ class ReflectionLibraryLoader extends LibraryLoader { @Override - T loadLibrary(NativeLibrary library, Class interfaceClass, Map libraryOptions, boolean failImmediately) { + T loadLibrary(NativeLibrary library, Class interfaceClass, MethodHandles.Lookup lookup, Map libraryOptions, boolean failImmediately) { Map invokers = new LazyLoader(library, interfaceClass, libraryOptions); if (failImmediately) { @@ -61,6 +62,7 @@ T loadLibrary(NativeLibrary library, Class interfaceClass, Map T loadLibrary(NativeLibrary library, Class interfaceClass, Map Date: Mon, 14 Jun 2021 15:52:15 -0500 Subject: [PATCH 2/2] Test for default methods on interface See #249. --- src/test/java/jnr/ffi/InvocationTest.java | 17 +++++++++++++++++ src/test/java/jnr/ffi/TstUtil.java | 14 ++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/test/java/jnr/ffi/InvocationTest.java b/src/test/java/jnr/ffi/InvocationTest.java index c8ecadafa..c7050f353 100644 --- a/src/test/java/jnr/ffi/InvocationTest.java +++ b/src/test/java/jnr/ffi/InvocationTest.java @@ -21,6 +21,8 @@ import org.junit.BeforeClass; import org.junit.Test; +import java.lang.invoke.MethodHandles; + import static org.junit.Assert.assertEquals; /** @@ -31,6 +33,14 @@ public static interface TestLib { int ret_int32_t(int i); } + public static interface TestLibWithDefault { + static final MethodHandles.Lookup lookup = MethodHandles.lookup(); + int ret_int32_t(int i); + default int doesNotExist(int i) { + return ret_int32_t(i); + } + } + static TestLib testlib; @BeforeClass @@ -44,4 +54,11 @@ public void hammer() throws Throwable { assertEquals(i, testlib.ret_int32_t(i)); } } + + @Test + public void testDefault() throws Throwable { + TestLibWithDefault withDefault = TstUtil.loadTestLib(TestLibWithDefault.class, TestLibWithDefault.lookup); + + assertEquals(testlib.ret_int32_t(1234), withDefault.doesNotExist(1234)); + } } diff --git a/src/test/java/jnr/ffi/TstUtil.java b/src/test/java/jnr/ffi/TstUtil.java index b07730ad3..76523270f 100644 --- a/src/test/java/jnr/ffi/TstUtil.java +++ b/src/test/java/jnr/ffi/TstUtil.java @@ -20,6 +20,7 @@ import jnr.ffi.provider.FFIProvider; +import java.lang.invoke.MethodHandles; import java.nio.ByteBuffer; import java.util.Collections; import java.util.Map; @@ -42,11 +43,20 @@ public static interface HelperLib { } public static T loadTestLib(Class interfaceClass) { + return loadTestLib(interfaceClass, (MethodHandles.Lookup) null); + } + + public static T loadTestLib(Class interfaceClass, MethodHandles.Lookup lookup) { final Map options = Collections.emptyMap(); - return loadTestLib(interfaceClass, options); + return loadTestLib(interfaceClass, lookup, options); } + public static T loadTestLib(Class interfaceClass, Map options) { - LibraryLoader loader = (provider != null ? provider : FFIProvider.getSystemProvider()).createLibraryLoader(interfaceClass); + return loadTestLib(interfaceClass, null, options); + } + + public static T loadTestLib(Class interfaceClass, MethodHandles.Lookup lookup, Map options) { + LibraryLoader loader = (provider != null ? provider : FFIProvider.getSystemProvider()).createLibraryLoader(interfaceClass, lookup); loader.library(libname); for (Map.Entry option : options.entrySet()) {