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.
This commit is contained in:
Luca Longinotti 2013-06-12 13:38:10 +02:00
parent a47e3cd3c3
commit 275bc37a77
3 changed files with 65 additions and 38 deletions

View File

@ -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);
}
/**

View File

@ -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<Integer, ImmutablePair<PollfdListener, Object>> pollfdListeners =
new ConcurrentHashMap<Integer, ImmutablePair<PollfdListener, Object>>();
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<PollfdListener, Object>(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<PollfdListener, Object> 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<PollfdListener, Object> 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

View File

@ -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);