Adds support for a custom SymbolProvider in NativeLibrary & Library

[soywiz]

Retake on #1483 I'm using a different branch, since I used master and that was problematic for syncing the original repo. I have also squashed all the commits into one.

---

This allows to do function hooking/replacement while still using direct mapping.

In addition to that, to properly using the OpenGL library on Windows, and access OpenGL >= 2.0, we need to use a custom getSymbol function: https://learn.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-wglgetprocaddress

In the test I'm switching cos and sin functions to show it works.

---

Started a thread here: https://groups.google.com/g/jna-users/c/RDt8sHAnTm0

[soywiz]

@matthiasblaesing can this be considered? is there something else I can do to move this forward?

[matthiasblaesing]

@soywiz I am/was busy with other things. I'll look into it.

[soywiz]

Sure. No problem and no rush. Just wanted to know if I missed something. Thanks for letting me know!

[matthiasblaesing]

Looks ok in general to me. What I noticed is:

• This change also affects the InvocationHandler codepath, so this also needs to have a test for the non-direct invocation case.
• The naming of the third parameter in SymbolProvider#getSymbolAddress makes me wonder if parent is really right here. Though I have not really reached a good conclusion
• Please recheck the commit and ensure, that your real name is in the author field (git log -n 1 is your friend)

For the change in NativeLibrary - what do you think about this:

--- a/src/com/sun/jna/NativeLibrary.java
+++ b/src/com/sun/jna/NativeLibrary.java
@@ -87,17 +87,22 @@
 public class NativeLibrary implements Closeable {
 
     private static final Logger LOG = Logger.getLogger(NativeLibrary.class.getName());
-    private final static Level DEBUG_LOAD_LEVEL = DEBUG_LOAD ? Level.INFO : Level.FINE;
+    private static final Level DEBUG_LOAD_LEVEL = DEBUG_LOAD ? Level.INFO : Level.FINE;
+    private static final SymbolProvider NATIVE_SYMBOL_PROVIDER = new SymbolProvider() {
+        @Override
+        public long getSymbolAddress(long handle, String name, SymbolProvider parent) {
+            return Native.findSymbol(handle, name);
+        }
+    };
 
     private Cleaner.Cleanable cleanable;
     private long handle;
     private final String libraryName;
     private final String libraryPath;
     private final Map<String, Function> functions = new HashMap<String, Function>();
+    private final SymbolProvider symbolProvider;
     final int callFlags;
     private String encoding;
-    private SymbolProvider symbolProvider;
-    private SymbolProvider defaultSymbolProvider;
     final Map<String, ?> options;
 
     private static final Map<String, Reference<NativeLibrary>> libraries = new HashMap<String, Reference<NativeLibrary>>();
@@ -125,15 +130,11 @@
         this.callFlags = callingConvention;
         this.options = options;
         this.encoding = (String)options.get(Library.OPTION_STRING_ENCODING);
-        this.symbolProvider = (SymbolProvider)options.get(Library.OPTION_SYMBOL_PROVIDER);
-        if (this.symbolProvider != null) {
-            //noinspection Convert2Lambda
-            defaultSymbolProvider = new SymbolProvider() {
-                @Override
-                public long getSymbolAddress(long handle, String name, SymbolProvider parent) {
-                    return Native.findSymbol(handle, name);
-                }
-            };
+        SymbolProvider optionSymbolProvider = (SymbolProvider)options.get(Library.OPTION_SYMBOL_PROVIDER);
+        if (optionSymbolProvider == null) {
+            this.symbolProvider = NATIVE_SYMBOL_PROVIDER;
+        } else {
+            this.symbolProvider = optionSymbolProvider;
         }
 
         if (this.encoding == null) {
@@ -649,11 +650,7 @@
         if (handle == 0) {
             throw new UnsatisfiedLinkError("Library has been unloaded");
         }
-        if (symbolProvider != null) {
-            return symbolProvider.getSymbolAddress(handle, name, defaultSymbolProvider);
-        } else {
-            return Native.findSymbol(handle, name);
-        }
+        return this.symbolProvider.getSymbolAddress(handle, name, NATIVE_SYMBOL_PROVIDER);
     }
 
     @Override

[soywiz]

@matthiasblaesing applied your patch since it makes sense to me. Also added my real name in the git commit.

Regarding to "InvocationHandler codepath" I don't know what that really means. I mean: for me there are two code paths, the default one, and the one that provides a custom OPTION_SYMBOL_PROVIDER. The default one was already being tested, and the new one is also tested in the PR. And I can't see any InvocationHandler reference in the PR, so I think I need a small clarification on that.

[matthiasblaesing]

@matthiasblaesing applied your patch since it makes sense to me. Also added my real name in the git commit.

Regarding to "InvocationHandler codepath" I don't know what that really means. I mean: for me there are two code paths, the default one, and the one that provides a custom OPTION_SYMBOL_PROVIDER. The default one was already being tested, and the new one is also tested in the PR. And I can't see any InvocationHandler reference in the PR, so I think I need a small clarification on that.

I mean this construct (untested!):

    interface MathInterfaceWithSymbolProvider extends Library {

        public MathInterfaceWithSymbolProvider INSTANCE = Native.load(
                Platform.MATH_LIBRARY_NAME,
                MathInterfaceWithSymbolProvider.class, Collections.singletonMap(
                        Library.OPTION_SYMBOL_PROVIDER,
                        new SymbolProvider() {
                            @Override
                            public long getSymbolAddress(long handle, String name, SymbolProvider parent) {
                                if (name.equals("sin")) {
                                    return parent.getSymbolAddress(handle, "cos", null);
                                } else {
                                    return parent.getSymbolAddress(handle, "sin", null);
                                }
                            }
                        }
                )
        );

        double cos(double x);
        double sin(double x);
    }

Native#load takes an interface, that defines the mappings to the native functions as abstract methods. For this interface a java.lang.reflect.Proxy is created, which delegates invocations to an (java.lang.reflect.InvocationHandler).

The implementation can be found here:

jna/src/com/sun/jna/Library.java

Lines 117 to 268 in 9e7160e

	static class Handler implements InvocationHandler {
	static final Method OBJECT_TOSTRING;
	static final Method OBJECT_HASHCODE;
	static final Method OBJECT_EQUALS;
	static {
	try {
	OBJECT_TOSTRING = Object.class.getMethod("toString");
	OBJECT_HASHCODE= Object.class.getMethod("hashCode");
	OBJECT_EQUALS = Object.class.getMethod("equals", Object.class);
	} catch (Exception e) {
	throw new Error("Error retrieving Object.toString() method");
	}
	}
	/**
	* FunctionInfo has to be immutable to to make the object visible
	* to other threads fully initialized. This is a prerequisite for
	* using the class in the double checked locking scenario of {@link Handler#invoke(Object, Method, Object[])}
	*/
	private static final class FunctionInfo {
	final InvocationHandler handler;
	final Function function;
	final boolean isVarArgs;
	final Object methodHandle;
	final Map<String, ?> options;
	final Class<?>[] parameterTypes;
	FunctionInfo(Object mh) {
	this.handler = null;
	this.function = null;
	this.isVarArgs = false;
	this.options = null;
	this.parameterTypes = null;
	this.methodHandle = mh;
	}
	FunctionInfo(InvocationHandler handler, Function function, Class<?>[] parameterTypes, boolean isVarArgs, Map<String, ?> options) {
	this.handler = handler;
	this.function = function;
	this.isVarArgs = isVarArgs;
	this.options = options;
	this.parameterTypes = parameterTypes;
	this.methodHandle = null;
	}
	}
	private final NativeLibrary nativeLibrary;
	private final Class<?> interfaceClass;
	// Library invocation options
	private final Map<String, Object> options;
	private final InvocationMapper invocationMapper;
	private final Map<Method, FunctionInfo> functions = new WeakHashMap<Method, FunctionInfo>();
	public Handler(String libname, Class<?> interfaceClass, Map<String, ?> options) {
	if (libname != null && "".equals(libname.trim())) {
	throw new IllegalArgumentException("Invalid library name \"" + libname + "\"");
	}
	if (!interfaceClass.isInterface()) {
	throw new IllegalArgumentException(libname + " does not implement an interface: " + interfaceClass.getName());
	}
	this.interfaceClass = interfaceClass;
	this.options = new HashMap<String, Object>(options);
	int callingConvention = AltCallingConvention.class.isAssignableFrom(interfaceClass)
	? Function.ALT_CONVENTION
	: Function.C_CONVENTION;
	if (this.options.get(OPTION_CALLING_CONVENTION) == null) {
	this.options.put(OPTION_CALLING_CONVENTION, Integer.valueOf(callingConvention));
	}
	if (this.options.get(OPTION_CLASSLOADER) == null) {
	this.options.put(OPTION_CLASSLOADER, interfaceClass.getClassLoader());
	}
	this.nativeLibrary = NativeLibrary.getInstance(libname, this.options);
	invocationMapper = (InvocationMapper)this.options.get(OPTION_INVOCATION_MAPPER);
	}
	public NativeLibrary getNativeLibrary() {
	return nativeLibrary;
	}
	public String getLibraryName() {
	return nativeLibrary.getName();
	}
	public Class<?> getInterfaceClass() {
	return interfaceClass;
	}
	@Override
	public Object invoke(Object proxy, Method method, Object[] inArgs)
	throws Throwable {
	// Intercept Object methods
	if (OBJECT_TOSTRING.equals(method)) {
	return "Proxy interface to " + nativeLibrary;
	} else if (OBJECT_HASHCODE.equals(method)) {
	return Integer.valueOf(hashCode());
	} else if (OBJECT_EQUALS.equals(method)) {
	Object o = inArgs[0];
	if (o != null && Proxy.isProxyClass(o.getClass())) {
	return Function.valueOf(Proxy.getInvocationHandler(o) == this);
	}
	return Boolean.FALSE;
	}
	// Using the double-checked locking pattern to speed up function calls
	FunctionInfo f = functions.get(method);
	if(f == null) {
	synchronized(functions) {
	f = functions.get(method);
	if (f == null) {
	boolean isDefault = ReflectionUtils.isDefault(method);
	if(! isDefault) {
	boolean isVarArgs = Function.isVarArgs(method);
	InvocationHandler handler = null;
	if (invocationMapper != null) {
	handler = invocationMapper.getInvocationHandler(nativeLibrary, method);
	}
	Function function = null;
	Class<?>[] parameterTypes = null;
	Map<String, Object> options = null;
	if (handler == null) {
	// Find the function to invoke
	function = nativeLibrary.getFunction(method.getName(), method);
	parameterTypes = method.getParameterTypes();
	options = new HashMap<String, Object>(this.options);
	options.put(Function.OPTION_INVOKING_METHOD, method);
	}
	f = new FunctionInfo(handler, function, parameterTypes, isVarArgs, options);
	} else {
	f = new FunctionInfo(ReflectionUtils.getMethodHandle(method));
	}
	functions.put(method, f);
	}
	}
	}
	if (f.methodHandle != null) {
	return ReflectionUtils.invokeDefaultMethod(proxy, f.methodHandle, inArgs);
	} else {
	if (f.isVarArgs) {
	inArgs = Function.concatenateVarArgs(inArgs);
	}
	if (f.handler != null) {
	return f.handler.invoke(proxy, method, inArgs);
	}
	return f.function.invoke(method, f.parameterTypes, method.getReturnType(), inArgs, f.options);
	}
	}
	}

HTH
Matthias

[soywiz]

I see. So I guess incidencially this should be usable with the interface variant too. I have updated the title of the PR, updated the README and added a test so the scope of this PR is properly described

[matthiasblaesing]

Final nitpick: Please move the tests together into NativeCustomSymbolProviderTest. That way the test is focus on a single feature and not spread over two classes.

[soywiz]

@matthiasblaesing Done.

[matthiasblaesing]

Change looks good to me and Appveyor, Travis and github actions agree. Thank you.