revise thread termination flag logic

native/Makefile

@@ -48,7 +48,7 @@ OS=$(shell uname | sed -e 's/CYGWIN.*/win32/g' \
                         -e 's/Linux.*/linux/g')
 
 JNA_JNI_VERSION=4.0.0 # auto-generated by ant
-CHECKSUM=059b6e5f0534df9b7f28dd7a87485721 # auto-generated by ant
+CHECKSUM=1a6047467b59e8748f975e03016ce3d9 # auto-generated by ant
 
 JAVA_INCLUDES=-I"$(JAVA_HOME)/include" \
               -I"$(JAVA_HOME)/include/$(OS)"

native/callback.c

@@ -81,6 +81,7 @@ static void * const dll_fptrs[] = {
 typedef struct _tls {
   JavaVM* jvm;
   jint last_error;
+  // Contents set to JNI_TRUE if thread has terminated and detached properly
   int* termination_flag;
   jboolean detach;
   char name[256];
@@ -503,6 +504,8 @@ static thread_storage* get_thread_storage(JNIEnv* env) {
     }
     else {
       snprintf(tls->name, sizeof(tls->name), "<uninitialized thread name>");
+      tls->last_error = 0;
+      tls->termination_flag = NULL;
       if ((*env)->GetJavaVM(env, &tls->jvm) != JNI_OK) {
         free(tls);
         throwByName(env, EIllegalState, "JNA: Could not get JavaVM");
@@ -523,12 +526,14 @@ static void dispose_thread_data(void* data) {
   JavaVM* jvm = tls->jvm;
   JNIEnv* env;
   int is_attached = (*jvm)->GetEnv(jvm, (void*)&env, JNI_VERSION_1_4) == JNI_OK;
+  jboolean detached = JNI_TRUE;
   if (is_attached) {
     if ((*jvm)->DetachCurrentThread(jvm) != 0) {
       fprintf(stderr, "JNA: could not detach native thread (automatic)\n");
+      detached = JNI_FALSE;
     }
   }
-  if (tls->termination_flag) {
+  if (tls->termination_flag && detached) {
     *(tls->termination_flag) = JNI_TRUE;
   }
   free(data);
@@ -572,10 +577,11 @@ static void make_thread_data_key() {
 
 /** Store the requested detach state for the current thread. */
 void
-JNA_detach(JNIEnv* env, jboolean d) {
+JNA_detach(JNIEnv* env, jboolean d, void* termination_flag) {
   thread_storage* tls = get_thread_storage(env);
   if (tls) {
     tls->detach = d;
+    tls->termination_flag = (int *)termination_flag;
   }
 }
 
@@ -611,7 +617,6 @@ callback_dispatch(ffi_cif* cif, void* resp, void** cbargs, void* user_data) {
     int attach_status = 0;
     JavaVMAttachArgs args;
     int daemon = JNI_FALSE;
-    int* termination_flag = NULL;
 
     args.version = JNI_VERSION_1_2;
     args.name = NULL;
@@ -625,7 +630,6 @@ callback_dispatch(ffi_cif* cif, void* resp, void** cbargs, void* user_data) {
       daemon = options.daemon ? JNI_TRUE : JNI_FALSE;
       detach = options.detach ? JNI_TRUE : JNI_FALSE;
       args.name = options.name;
-      termination_flag = options.termination_flag;
     }
     if (daemon) {
       attach_status = (*jvm)->AttachCurrentThreadAsDaemon(jvm, (void*)&env, &args);
@@ -635,8 +639,8 @@ callback_dispatch(ffi_cif* cif, void* resp, void** cbargs, void* user_data) {
     }
     tls = get_thread_storage(env);
     if (tls) {
-      strncpy(tls->name, args.name ? args.name : "<unconfigured thread>", sizeof(tls->name));
-      tls->termination_flag = termination_flag;
+      strncpy(tls->name, args.name ? args.name : "<unconfigured native thread>", sizeof(tls->name));
+      tls->detach = detach;
     }
     // Dispose of allocated memory
     free(args.name);
@@ -660,9 +664,11 @@ callback_dispatch(ffi_cif* cif, void* resp, void** cbargs, void* user_data) {
     fprintf(stderr, "JNA: Out of memory: Can't allocate local frame\n");
   }
   else {
-    tls->detach = detach;
     callback_invoke(env, cb, cif, resp, cbargs);
-    detach = tls->detach;
+    // Make note of whether the callback wants to avoid detach
+    if (!tls->detach) {
+      detach = JNI_FALSE;
+    }
     (*env)->PopLocalFrame(env, NULL);
   }
 

native/dispatch.c

@@ -3298,8 +3298,8 @@ Java_com_sun_jna_Native_initialize_1ffi_1type(JNIEnv *env, jclass UNUSED(cls), j
 }
 
 JNIEXPORT void JNICALL
-Java_com_sun_jna_Native_detach(JNIEnv* env, jclass UNUSED(cls), jboolean d) {
-  JNA_detach(env, d);
+Java_com_sun_jna_Native_setDetachState(JNIEnv* env, jclass UNUSED(cls), jboolean d, jlong flag) {
+  JNA_detach(env, d, L2A(flag));
 }
 
 #ifdef __cplusplus

native/dispatch.h

@@ -182,7 +182,7 @@ extern const char* JNA_callback_init(JNIEnv*);
 extern void JNA_set_last_error(JNIEnv*,int);
 extern int JNA_get_last_error(JNIEnv*);
 extern void JNA_callback_dispose(JNIEnv*);
-extern void JNA_detach(JNIEnv*,jboolean);
+extern void JNA_detach(JNIEnv*,jboolean,void*);
 extern callback* create_callback(JNIEnv*, jobject, jobject,
                                  jobjectArray, jclass,
                                  callconv_t, jint, jstring);
@@ -214,7 +214,6 @@ typedef struct _AttachOptions {
   int daemon;
   int detach;
   char* name;
-  int* termination_flag;
 } AttachOptions;
 extern jobject initializeThread(callback*,AttachOptions*);
 

src/com/sun/jna/CallbackReference.java

@@ -38,7 +38,6 @@ class CallbackReference extends WeakReference {
     static final Map callbackMap = new WeakHashMap();
     static final Map directCallbackMap = new WeakHashMap();
     static final Map allocations = new WeakHashMap();
-    static final Map nativeThreads = Collections.synchronizedMap(new WeakHashMap());
 
     private static final Method PROXY_CALLBACK_METHOD;
 
@@ -67,32 +66,20 @@ static class AttachOptions extends Structure {
         public boolean daemon;
         public boolean detach;
         public String name;
-        public Pointer termination_flag;
         // Thread name must be UTF8-encoded
         { setStringEncoding("utf8"); }
         protected List getFieldOrder() {
-            return Arrays.asList(new String[] { "daemon", "detach", "name", "termination_flag" });
+            return Arrays.asList(new String[] { "daemon", "detach", "name", });
         }
     }
 
-    private static ThreadLocal terminationFlag = new ThreadLocal() {
-        protected Object initialValue() {
-            return new Memory(4);
-        }
-    };
-
-    /** Returns the termination flag associated with the given thread. */
-    static Pointer getTerminationFlag(Thread t) {
-        return (Pointer)nativeThreads.get(t);
-    }
-
     /** Called from native code to initialize a callback thread. */
     private static ThreadGroup initializeThread(Callback cb, AttachOptions args) {
         CallbackThreadInitializer init = null;
         if (cb instanceof DefaultCallbackProxy) {
             cb = ((DefaultCallbackProxy)cb).getCallback();
         }
-        synchronized(initializers) {
+        synchronized(callbackMap) {
             init = (CallbackThreadInitializer)initializers.get(cb);
         }
         ThreadGroup group = null;
@@ -101,11 +88,8 @@ private static ThreadGroup initializeThread(Callback cb, AttachOptions args) {
             args.name = init.getName(cb);
             args.daemon = init.isDaemon(cb);
             args.detach = init.detach(cb);
+            args.write();
         }
-        args.termination_flag = (Pointer)terminationFlag.get();
-        args.termination_flag.setInt(0, 0);
-        args.write();
-        nativeThreads.put(Thread.currentThread(), args.termination_flag);
         return group;
     }
 

src/com/sun/jna/Native.java

@@ -1838,9 +1838,28 @@ static String getString(long addr, String encoding) {
         <em>Warning</em>: avoid calling {@link #detach detach(true)} on threads
         spawned by the JVM; the resulting behavior is not defined.
      */
-    // TODO: keep references to Java non-detached threads, and clear them when
-    // native side sets a flag saying they're detached (cleanup)
-    public static native void detach(boolean detach);
+    public static void detach(boolean detach) {
+        Pointer p = (Pointer)nativeThreadTerminationFlag.get();
+        nativeThreads.put(Thread.currentThread(), p);
+        setDetachState(detach, p.peer);
+    }
+
+    static Pointer getTerminationFlag(Thread t) {
+        return (Pointer)nativeThreads.get(t);
+    }
+
+    private static Map nativeThreads = Collections.synchronizedMap(new WeakHashMap());
+
+    private static ThreadLocal nativeThreadTerminationFlag = 
+        new ThreadLocal() {
+            protected Object initialValue() {
+                Memory m = new Memory(4);
+                m.clear();
+                return m;
+            }
+        };
+
+    private static native void setDetachState(boolean detach, long terminationFlag);
 
     private static class Buffers {
         static boolean isBuffer(Class cls) {

test/com/sun/jna/CallbacksTest.java

@@ -1159,7 +1159,8 @@ public String getName(Callback cb) {
 	    Thread.enumerate(remaining);
             if (System.currentTimeMillis() - start > 10000) {
                 Thread t = (Thread)ref.get();
-                Pointer terminationFlag = CallbackReference.getTerminationFlag(t);
+                Pointer terminationFlag = Native.getTerminationFlag(t);
+                assertNotNull("Native thread termination flag is missing", terminationFlag);
                 if (terminationFlag.getInt(0) == 0) {
                     fail("Timed out waiting for native attached thread to be GC'd: " + t + " alive: "
                          + t.isAlive() + " daemon: " + t.isDaemon() + "\n" + Arrays.asList(remaining));