Pointer already mapped

CHANGES.md

@@ -16,6 +16,7 @@ Features
 
 Bug Fixes
 ---------
+* [#326](https://github.com/java-native-access/jna/issues/326): Fix loading library that re-uses pointers for different callbacks - [@fpapai](https://github.com/fpapai).
 * [#1244](https://github.com/java-native-access/jna/issues/1244): Fix building on GCC 10 - [@matthiasblaesing](https://github.com/matthiasblaesing).
 * [#1252](https://github.com/java-native-access/jna/issues/1252): - Fix bindings of `CTL_ENTRY#getRgAttribute`, `CTL_INFO#getRgCTLEntry`, `CTL_INFO#getRgExtension`, `CERT_EXTENSIONS#getRgExtension`, `CERT_INFO#getRgExtension`, `CRL_INFO#getRgCRLEntry`, `CRL_INFO#getRgExtension`, `CRL_ENTRY#getRgExtension`. Add bindings for `CertEnumCertificatesInStore`, `CertEnumCTLsInStore`, `CertEnumCRLsInStore` and `CryptQueryObject` in `c.s.j.p.win32.Crypt32`.<br> *WARNING:* The signatures for `CTL_INFO#getRgCTLEntry` and `CTL_INFO#getRgExtension` were changed - as the original signatures were obviously wrong and read the wrong attributes, it is not considered an API break - [@matthiasblaesing](https://github.com/matthiasblaesing).
 

src/com/sun/jna/CallbackReference.java

@@ -53,7 +53,8 @@ public class CallbackReference extends WeakReference<Callback> {
     // by synchonizing on pointerCallbackMap
     static final Map<Callback, CallbackReference> callbackMap = new WeakHashMap<Callback, CallbackReference>();
     static final Map<Callback, CallbackReference> directCallbackMap = new WeakHashMap<Callback, CallbackReference>();
-    static final Map<Pointer, Reference<Callback>> pointerCallbackMap = new WeakHashMap<Pointer, Reference<Callback>>();
+    //callbacks with different signatures sharing the same pointer
+    static final Map<Pointer, Reference<Callback>[]> pointerCallbackMap = new WeakHashMap<Pointer, Reference<Callback>[]>();
     // Track memory allocations associated with this closure (usually String args)
     static final Map<Object, Object> allocations =
             Collections.synchronizedMap(new WeakHashMap<Object, Object>());
@@ -143,31 +144,70 @@ private static Callback getCallback(Class<?> type, Pointer p, boolean direct) {
             throw new IllegalArgumentException("Callback type must be an interface");
         Map<Callback, CallbackReference> map = direct ? directCallbackMap : callbackMap;
         synchronized(pointerCallbackMap) {
-            Callback cb = null;
-            Reference<Callback> ref = pointerCallbackMap.get(p);
-            if (ref != null) {
-                cb = ref.get();
-                if (cb != null && !type.isAssignableFrom(cb.getClass())) {
-                    throw new IllegalStateException("Pointer " + p + " already mapped to " + cb
-                                                    + ".\nNative code may be re-using a default function pointer"
-                                                    + ", in which case you may need to use a common Callback class"
-                                                    + " wherever the function pointer is reused.");
-                }
+            Reference<Callback>[] array = pointerCallbackMap.get(p);
+            Callback cb = getTypeAssignableCallback(type, array);
+            if (cb != null) {
                 return cb;
             }
-            int ctype = AltCallingConvention.class.isAssignableFrom(type)
-                ? Function.ALT_CONVENTION : Function.C_CONVENTION;
-            Map<String, Object> foptions = new HashMap<String, Object>(Native.getLibraryOptions(type));
-            foptions.put(Function.OPTION_INVOKING_METHOD, getCallbackMethod(type));
-            NativeFunctionHandler h = new NativeFunctionHandler(p, ctype, foptions);
-            cb = (Callback)Proxy.newProxyInstance(type.getClassLoader(), new Class[] { type }, h);
+            cb = createCallback(type, p);
+            pointerCallbackMap.put(p, addCallbackToArray(cb,array));
+
             // No CallbackReference for this callback
             map.remove(cb);
-            pointerCallbackMap.put(p, new WeakReference<Callback>(cb));
             return cb;
         }
     }
 
+    private static Callback getTypeAssignableCallback(Class<?> type, Reference<Callback>[] array) {
+        if (array != null) {
+            for (int i=0;i < array.length;i++) {
+                Callback cb = array[i].get();
+                if (cb != null && type.isAssignableFrom(cb.getClass())) {
+                    return cb;
+                }
+            }
+        }
+        return null;
+    }
+
+
+    private static Reference<Callback>[] addCallbackToArray(Callback cb,Reference<Callback>[] array) {
+        int reqArraySize = 1; //space for the new item
+        if (array != null) {
+            //drop any freed reference
+            for (int i=0;i < array.length;i++) {
+                if( array[i].get() == null ) {
+                    array[i] = null;
+                }
+                else {
+                    reqArraySize++;
+                }
+            }
+        }
+        @SuppressWarnings( "unchecked" )
+        Reference<Callback>[] newArray = new Reference[reqArraySize];
+        int nidx=0;
+        if (array != null) {
+            //shift items if needed
+            for (int i=0;i < array.length;i++) {
+                if (array[i] != null) {
+                    newArray[nidx++] = array[i];
+                }
+            }
+        }
+        newArray[nidx] = new WeakReference<Callback>(cb);
+        return newArray;
+    }
+
+    private static Callback createCallback(Class<?> type, Pointer p) {
+        int ctype = AltCallingConvention.class.isAssignableFrom(type)
+                ? Function.ALT_CONVENTION : Function.C_CONVENTION;
+        Map<String, Object> foptions = new HashMap<String, Object>(Native.getLibraryOptions(type));
+        foptions.put(Function.OPTION_INVOKING_METHOD, getCallbackMethod(type));
+        NativeFunctionHandler h = new NativeFunctionHandler(p, ctype, foptions);
+        return (Callback)Proxy.newProxyInstance(type.getClassLoader(), new Class[] { type }, h);
+    }
+
     Pointer cbstruct;
     Pointer trampoline;
     // Keep a reference to the proxy to avoid premature GC of it
@@ -451,7 +491,9 @@ private static Pointer getFunctionPointer(Callback cb, boolean direct) {
             if (cbref == null) {
                 cbref = new CallbackReference(cb, callingConvention, direct);
                 map.put(cb, cbref);
-                pointerCallbackMap.put(cbref.getTrampoline(), new WeakReference<Callback>(cb));
+                pointerCallbackMap.put(cbref.getTrampoline(),
+                        addCallbackToArray(cb, null));
+
                 if (initializers.containsKey(cb)) {
                     cbref.setCallbackOptions(Native.CB_HAS_INITIALIZER);
                 }

test/com/sun/jna/CallbacksTest.java

@@ -285,15 +285,16 @@ public void testLookupNonCallbackClass() {
         }
     }
 
-    public void testThrowOnMultiplyMappedCallback() {
-        try {
-            Pointer p = new Pointer(getName().hashCode());
-            CallbackReference.getCallback(TestLibrary.VoidCallback.class, p);
-            CallbackReference.getCallback(TestLibrary.ByteCallback.class, p);
-            fail("Multiply-mapped callback should fail");
-        }
-        catch(IllegalStateException e) {
-        }
+    public void testAcceptMultiplyMappedCallbacks() {
+        Pointer p = new Pointer(getName().hashCode());
+
+        Callback cbV1 = CallbackReference.getCallback(TestLibrary.VoidCallback.class, p);
+        Callback cbB1 = CallbackReference.getCallback(TestLibrary.ByteCallback.class, p);
+        Callback cbV2 = CallbackReference.getCallback(TestLibrary.VoidCallback.class, p);
+        Callback cbB2 = CallbackReference.getCallback(TestLibrary.ByteCallback.class, p);
+
+        assertSame(cbV1, cbV2);
+        assertSame(cbB1, cbB2);
     }
 
     public void testNoMethodCallback() {