From 275bc37a77903c2635feb49a408bb818ff7aefe2 Mon Sep 17 00:00:00 2001 From: Luca Longinotti Date: Wed, 12 Jun 2013 13:38:10 +0200 Subject: [PATCH] Right now if you add different PollfdListeners to different contexts, only the last one will be remembered, and subsequent callbacks, even from other contexts, will always use that last one. This is obviously unexpected and incorrect, yet cannot be fixed by just adding more data on the C side: you'd have to remember the callback object at the very least, which you could in the user_data field, but you'd have to make it a global reference. Yet, there would be no easy way to later correctly delete that global reference, since to unset the pollfdNotifiers you just pass NULL to the same function again, and you don't get back any data on what was there before. The easiest way to fix this was thus to change to a HashMap, Concurrent since access can be from random threads. The key was chosen to be the Context.hashCode(), as Integer lookup and comparison is fast, and the hashCode depends directly on the contextPointer stored in the Context (which identifies it). This works for sure on 32bit systems, but I'm still concerned about 64bit systems, where a long would probably make more sense, I need to think about it some more. --- src/main/c/src/LibUsb.c | 23 ++++--- .../java/de/ailis/usb4java/libusb/LibUsb.java | 62 +++++++++++++------ .../de/ailis/usb4java/libusb/LibUSBTest.java | 18 +++--- 3 files changed, 65 insertions(+), 38 deletions(-) diff --git a/src/main/c/src/LibUsb.c b/src/main/c/src/LibUsb.c index fc762ae..15d8a30 100644 --- a/src/main/c/src/LibUsb.c +++ b/src/main/c/src/LibUsb.c @@ -1047,12 +1047,13 @@ JNIEXPORT jint JNICALL METHOD_NAME(LibUsb, getNextTimeout) jmethodID method = (*env)->GetMethodID(env, cls, "put", "(IJ)Ljava/nio/LongBuffer;"); (*env)->CallObjectMethod(env, timeout, method, 0, - tv.tv_sec * 1000000 + tv.tv_usec); + (jlong) (tv.tv_sec * 1000000 + tv.tv_usec)); } return result; } -static void LIBUSB_CALL triggerPollfdAdded(int fd, short events, void *user_data) +static void LIBUSB_CALL triggerPollfdAdded(int fd, short events, + void *user_data) { THREAD_BEGIN(env) @@ -1061,9 +1062,10 @@ static void LIBUSB_CALL triggerPollfdAdded(int fd, short events, void *user_data jobject object = (*env)->NewObject(env, fdcls, constructor, fd); jclass cls = (*env)->FindClass(env, PACKAGE_DIR"/LibUsb"); - jmethodID method = (*env)->GetStaticMethodID(env, cls, - "triggerPollfdAdded", "(Ljava/io/FileDescriptor;I)V"); - (*env)->CallStaticVoidMethod(env, cls, method, object, events); + jmethodID method = (*env)->GetStaticMethodID(env, cls, "triggerPollfdAdded", + "(Ljava/io/FileDescriptor;II)V"); + (*env)->CallStaticVoidMethod(env, cls, method, object, (jint) events, + (jint) (intptr_t) user_data); THREAD_END } @@ -1078,25 +1080,26 @@ static void LIBUSB_CALL triggerPollfdRemoved(int fd, void *user_data) jclass cls = (*env)->FindClass(env, PACKAGE_DIR"/LibUsb"); jmethodID method = (*env)->GetStaticMethodID(env, cls, - "triggerPollfdRemoved", "(Ljava/io/FileDescriptor;)V"); - (*env)->CallStaticVoidMethod(env, cls, method, object); + "triggerPollfdRemoved", "(Ljava/io/FileDescriptor;I)V"); + (*env)->CallStaticVoidMethod(env, cls, method, object, + (jint) (intptr_t) user_data); THREAD_END } /** - * void setPollfdNotifiers(Context) + * void setPollfdNotifiers(Context, int) */ JNIEXPORT void JNICALL METHOD_NAME(LibUsb, setPollfdNotifiers) ( - JNIEnv *env, jclass class, jobject context + JNIEnv *env, jclass class, jobject context, jint context_hash ) { libusb_context *ctx = unwrapContext(env, context); if (!ctx && context) return; libusb_set_pollfd_notifiers(ctx, &triggerPollfdAdded, &triggerPollfdRemoved, - NULL); + (void *) (intptr_t) context_hash); } /** diff --git a/src/main/java/de/ailis/usb4java/libusb/LibUsb.java b/src/main/java/de/ailis/usb4java/libusb/LibUsb.java index efefb9b..871f797 100644 --- a/src/main/java/de/ailis/usb4java/libusb/LibUsb.java +++ b/src/main/java/de/ailis/usb4java/libusb/LibUsb.java @@ -15,6 +15,10 @@ import java.io.FileDescriptor; import java.nio.ByteBuffer; import java.nio.IntBuffer; import java.nio.LongBuffer; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +import org.apache.commons.lang3.tuple.ImmutablePair; import de.ailis.usb4java.utils.BufferUtils; @@ -469,11 +473,11 @@ public final class LibUsb /** Device sent more data than requested. */ public static final int TRANSFER_OVERFLOW = 6; - /** The currently set pollfd listener. */ - private static PollfdListener pollfdListener; - - /** The currently set pollfd listener user data. */ - private static Object pollfdListenerUserData; + /** + * pollfd listeners (to support different listeners for different contexts). + */ + private static final ConcurrentMap> pollfdListeners = + new ConcurrentHashMap>(); static { @@ -1819,19 +1823,30 @@ public final class LibUsb public static void setPollfdNotifiers(final Context context, final PollfdListener listener, final Object userData) { - if (listener == null) + int contextHash; + + if (context == null) { - unsetPollfdNotifiers(context); + contextHash = 31; // Manual result of Context.hashCode() for 0. } else { - setPollfdNotifiers(context); + contextHash = context.hashCode(); } - // Once we know the native calls have gone through, update the - // references. - pollfdListener = listener; - pollfdListenerUserData = userData; + if (listener == null) + { + unsetPollfdNotifiers(context); + + pollfdListeners.remove(contextHash); + } + else + { + setPollfdNotifiers(context, contextHash); + + pollfdListeners.put(contextHash, + new ImmutablePair(listener, userData)); + } } /** @@ -1843,11 +1858,15 @@ public final class LibUsb * @param events * events to monitor for, see libusb_pollfd for a description */ - static void triggerPollfdAdded(final FileDescriptor fd, final int events) + static void triggerPollfdAdded(final FileDescriptor fd, final int events, + final int contextHash) { - if (pollfdListener != null) + final ImmutablePair listener = pollfdListeners + .get(contextHash); + + if (listener != null) { - pollfdListener.pollfdAdded(fd, events, pollfdListenerUserData); + listener.left.pollfdAdded(fd, events, listener.right); } } @@ -1857,11 +1876,15 @@ public final class LibUsb * @param fd * The removed file descriptor. */ - static void triggerPollfdRemoved(final FileDescriptor fd) + static void triggerPollfdRemoved(final FileDescriptor fd, + final int contextHash) { - if (pollfdListener != null) + final ImmutablePair listener = pollfdListeners + .get(contextHash); + + if (listener != null) { - pollfdListener.pollfdRemoved(fd, pollfdListenerUserData); + listener.left.pollfdRemoved(fd, listener.right); } } @@ -1872,7 +1895,8 @@ public final class LibUsb * @param context * The context to operate on, or NULL for the default context */ - static native void setPollfdNotifiers(final Context context); + static native void setPollfdNotifiers(final Context context, + final int contextHash); /** * Tells libusbx to stop informing this class about pollfd additions and diff --git a/src/test/java/de/ailis/usb4java/libusb/LibUSBTest.java b/src/test/java/de/ailis/usb4java/libusb/LibUSBTest.java index 7785e57..5526266 100644 --- a/src/test/java/de/ailis/usb4java/libusb/LibUSBTest.java +++ b/src/test/java/de/ailis/usb4java/libusb/LibUSBTest.java @@ -161,7 +161,7 @@ public class LibUSBTest assertNotNull(version); assertEquals(1, version.major()); assertEquals(0, version.minor()); - assertTrue(version.micro() > 0 && version.micro() < 100); + assertTrue((version.micro() > 0) && (version.micro() < 100)); assertNotNull(version.rc()); assertTrue(version.toString().startsWith("1.0.")); } @@ -176,7 +176,7 @@ public class LibUSBTest assumeUsbTestsEnabled(); assertEquals(LibUsb.SUCCESS, LibUsb.init(null)); LibUsb.exit(null); - + try { // Double-exit should throw exception @@ -186,7 +186,7 @@ public class LibUSBTest catch (IllegalStateException e) { // Expected behavior - } + } } /** @@ -200,7 +200,7 @@ public class LibUSBTest Context context = new Context(); assertEquals(LibUsb.SUCCESS, LibUsb.init(context)); LibUsb.exit(context); - + try { LibUsb.exit(context); @@ -880,7 +880,7 @@ public class LibUSBTest { assumeUsbTestsEnabled(); final Context context = new Context(); - LibUsb.setPollfdNotifiers(context); + LibUsb.setPollfdNotifiers(context, context.hashCode()); } /** @@ -910,7 +910,7 @@ public class LibUSBTest LibUsb.setPollfdNotifiers(context, listener, "test"); FileDescriptor fd = new FileDescriptor(); - LibUsb.triggerPollfdAdded(fd, 53); + LibUsb.triggerPollfdAdded(fd, 53, context.hashCode()); assertEquals(53, listener.addedEvents); assertSame(fd, listener.addedFd); assertSame("test", listener.addedUserData); @@ -920,7 +920,7 @@ public class LibUSBTest listener.reset(); fd = new FileDescriptor(); - LibUsb.triggerPollfdRemoved(fd); + LibUsb.triggerPollfdRemoved(fd, context.hashCode()); assertEquals(0, listener.addedEvents); assertNull(listener.addedFd); assertNull(listener.addedUserData); @@ -931,7 +931,7 @@ public class LibUSBTest listener.reset(); fd = new FileDescriptor(); - LibUsb.triggerPollfdAdded(fd, 53); + LibUsb.triggerPollfdAdded(fd, 53, context.hashCode()); assertEquals(0, listener.addedEvents); assertNull(listener.addedFd); assertNull(listener.addedUserData); @@ -941,7 +941,7 @@ public class LibUSBTest listener.reset(); fd = new FileDescriptor(); - LibUsb.triggerPollfdRemoved(fd); + LibUsb.triggerPollfdRemoved(fd, context.hashCode()); assertEquals(0, listener.addedEvents); assertNull(listener.addedFd); assertNull(listener.addedUserData);