You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by xl...@apache.org on 2007/08/01 13:11:16 UTC

svn commit: r561762 - in /harmony/enhanced/drlvm/trunk/vm: gc_gen/src/thread/mutator.cpp vmcore/include/finalizer_thread.h vmcore/include/ref_enqueue_thread.h vmcore/src/init/finalizer_thread.cpp vmcore/src/init/ref_enqueue_thread.cpp

Author: xli
Date: Wed Aug  1 04:11:15 2007
New Revision: 561762

URL: http://svn.apache.org/viewvc?view=rev&rev=561762
Log:
HARMONY-4456: fix a potential bug source assuming thread creation/destruction is not serialized

Modified:
    harmony/enhanced/drlvm/trunk/vm/gc_gen/src/thread/mutator.cpp
    harmony/enhanced/drlvm/trunk/vm/vmcore/include/finalizer_thread.h
    harmony/enhanced/drlvm/trunk/vm/vmcore/include/ref_enqueue_thread.h
    harmony/enhanced/drlvm/trunk/vm/vmcore/src/init/finalizer_thread.cpp
    harmony/enhanced/drlvm/trunk/vm/vmcore/src/init/ref_enqueue_thread.cpp

Modified: harmony/enhanced/drlvm/trunk/vm/gc_gen/src/thread/mutator.cpp
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/gc_gen/src/thread/mutator.cpp?view=diff&rev=561762&r1=561761&r2=561762
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/gc_gen/src/thread/mutator.cpp (original)
+++ harmony/enhanced/drlvm/trunk/vm/gc_gen/src/thread/mutator.cpp Wed Aug  1 04:11:15 2007
@@ -52,11 +52,10 @@
 
   mutator->next = (Mutator *)gc->mutator_list;
   gc->mutator_list = mutator;
+  gc->num_mutators++;
 
   unlock(gc->mutator_list_lock); // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   
-  gc->num_mutators++;
-  
   gc_set_tls(mutator);
   
   return;
@@ -73,16 +72,6 @@
   allocactor_destruct_local_chunks((Allocator*)mutator);
 #endif
 
-  if(gc_is_gen_mode()){ /* put back the remset when a mutator exits */
-    pool_put_entry(gc->metadata->mutator_remset_pool, mutator->rem_set);
-    mutator->rem_set = NULL;
-  }
-  
-  if(mutator->obj_with_fin){
-    pool_put_entry(gc->finref_metadata->obj_with_fin_pool, mutator->obj_with_fin);
-    mutator->obj_with_fin = NULL;
-  }
-
   lock(gc->mutator_list_lock);     // vvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
 
   volatile Mutator *temp = gc->mutator_list;
@@ -95,10 +84,19 @@
     }
     temp->next = mutator->next;
   }
+  gc->num_mutators--;
 
   unlock(gc->mutator_list_lock); // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-  gc->num_mutators--;
+  
+  if(gc_is_gen_mode()){ /* put back the remset when a mutator exits */
+    pool_put_entry(gc->metadata->mutator_remset_pool, mutator->rem_set);
+    mutator->rem_set = NULL;
+  }
+  
+  if(mutator->obj_with_fin){
+    pool_put_entry(gc->finref_metadata->obj_with_fin_pool, mutator->obj_with_fin);
+    mutator->obj_with_fin = NULL;
+  }
   
   //gc_set_tls(NULL);
   
@@ -124,5 +122,6 @@
   }  
   return;
 }
+
 
 

Modified: harmony/enhanced/drlvm/trunk/vm/vmcore/include/finalizer_thread.h
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/include/finalizer_thread.h?view=diff&rev=561762&r1=561761&r2=561762
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/vmcore/include/finalizer_thread.h (original)
+++ harmony/enhanced/drlvm/trunk/vm/vmcore/include/finalizer_thread.h Wed Aug  1 04:11:15 2007
@@ -43,17 +43,19 @@
 
 
 typedef struct Fin_Thread_Info {
-    hysem_t pending_sem;                        // finalizer pending event
+    hysem_t pending_sem;            // finalizer pending event
+    hysem_t attached_sem;           // fin thread attached event
     
-    hycond_t end_cond;                          // finalization end condition variable
-    hymutex_t end_mutex;                        // finalization end mutex
+    /* Using pair of cond and mutex rather than sem is because the waiting thread num is not constant */
+    hycond_t end_cond;              // finalization end condition variable
+    hymutex_t end_mutex;            // finalization end mutex
     
-    hycond_t mutator_block_cond;                // mutator block condition variable for heavy finalizable obj load
-    hymutex_t mutator_block_mutex;              // mutator block mutex for heavy finalizable obj load
+    /* Using pair of cond and mutex rather than sem is because the waiting thread num is not constant */
+    hycond_t mutator_block_cond;    // mutator block condition variable for heavy finalizable obj load
+    hymutex_t mutator_block_mutex;  // mutator block mutex for heavy finalizable obj load
     
     hythread_t *thread_ids;
     volatile unsigned int thread_num;
-    volatile unsigned int thread_attached;
     
     volatile Boolean shutdown;
     volatile Boolean on_exit;

Modified: harmony/enhanced/drlvm/trunk/vm/vmcore/include/ref_enqueue_thread.h
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/include/ref_enqueue_thread.h?view=diff&rev=561762&r1=561761&r2=561762
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/vmcore/include/ref_enqueue_thread.h (original)
+++ harmony/enhanced/drlvm/trunk/vm/vmcore/include/ref_enqueue_thread.h Wed Aug  1 04:11:15 2007
@@ -33,11 +33,15 @@
 
 
 typedef struct Ref_Enqueue_Thread_Info {
-    hysem_t pending_sem;
+    hysem_t pending_sem;    // weakref pending event
+    hysem_t attached_sem;   // ref enqueue thread attached event
+    
+    /* Using pair of cond and mutex rather than sem is because the waiting thread num is not constant */
     hycond_t end_cond;      // ref enqueue end condition variable
     hymutex_t end_mutex;    // ref enqueue end mutex
+    
     Boolean shutdown;
-    volatile unsigned int thread_attached;
+    volatile unsigned int thread_num;
     volatile unsigned int end_waiting_num;  // thread num waiting for finalization end
 }Ref_Enqueue_Thread_Info;
 

Modified: harmony/enhanced/drlvm/trunk/vm/vmcore/src/init/finalizer_thread.cpp
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/src/init/finalizer_thread.cpp?view=diff&rev=561762&r1=561761&r2=561762
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/vmcore/src/init/finalizer_thread.cpp (original)
+++ harmony/enhanced/drlvm/trunk/vm/vmcore/src/init/finalizer_thread.cpp Wed Aug  1 04:11:15 2007
@@ -63,25 +63,7 @@
     return 0;
 }
 
-static IDATA finalizer_thread_func(void **args);
-
-static void set_fin_thread_attached(void)
-{ fin_thread_info->thread_attached = 1; }
-
-static void dec_fin_thread_num(void)
-{ atomic_dec32(&fin_thread_info->thread_num); }
-
-static volatile unsigned int get_fin_thread_attached(void)
-{ return fin_thread_info->thread_attached; }
-
-static void clear_fin_thread_attached(void)
-{ fin_thread_info->thread_attached = 0; }
-
-static void wait_fin_thread_attached(void)
-{ while(!fin_thread_info->thread_attached){hythread_yield();}}
-
-
-jobject get_system_thread_group(JNIEnv* jni_env)
+jobject get_system_thread_group(JNIEnv *jni_env)
 {
     jclass thread_class = GetObjectClass(jni_env, jthread_self());
     jfieldID sysTG_field = GetStaticFieldID(jni_env, thread_class,
@@ -90,8 +72,11 @@
     return GetStaticObjectField(jni_env, thread_class, sysTG_field);
 }
 
-void finalizer_threads_init(JavaVM *java_vm, JNIEnv* jni_env)
-{   if(!native_fin_thread_flag)
+static IDATA finalizer_thread_func(void **args);
+
+void finalizer_threads_init(JavaVM *java_vm, JNIEnv *jni_env)
+{
+    if(!native_fin_thread_flag)
         return;
     
     fin_thread_info = (Fin_Thread_Info *)STD_MALLOC(sizeof(Fin_Thread_Info));
@@ -105,6 +90,9 @@
     IDATA status = hysem_create(&fin_thread_info->pending_sem, 0, fin_thread_info->thread_num);
     assert(status == TM_ERROR_NONE);
     
+    status = hysem_create(&fin_thread_info->attached_sem, 0, fin_thread_info->thread_num);
+    assert(status == TM_ERROR_NONE);
+    
     status = hycond_create(&fin_thread_info->end_cond);
     assert(status == TM_ERROR_NONE);
     status = hymutex_create(&fin_thread_info->end_mutex, TM_MUTEX_DEFAULT);
@@ -118,15 +106,13 @@
     fin_thread_info->thread_ids = (hythread_t *)STD_MALLOC(sizeof(hythread_t) * fin_thread_info->thread_num);
     
     for(unsigned int i = 0; i < fin_thread_info->thread_num; i++){
-        void **args = (void **)STD_MALLOC(sizeof(void *) * 3);
-        args[0] = (void *)java_vm;
-        args[1] = (void *)(UDATA)(i + 1);
-        args[2] = (void*)get_system_thread_group(jni_env);
+        void **args = (void **)STD_MALLOC(sizeof(void *) * 2);
+        args[0] = (void*)java_vm;
+        args[1] = (void*)get_system_thread_group(jni_env);
         fin_thread_info->thread_ids[i] = NULL;
-        clear_fin_thread_attached();
         status = hythread_create(&fin_thread_info->thread_ids[i], 0, FINALIZER_THREAD_PRIORITY, 0, (hythread_entrypoint_t)finalizer_thread_func, args);
         assert(status == TM_ERROR_NONE);
-        wait_fin_thread_attached();
+        hysem_wait(fin_thread_info->attached_sem);
     }    
 }
 
@@ -179,6 +165,7 @@
         atomic_dec32(&fin_thread_info->end_waiting_num);
         unsigned int temp = vm_get_finalizable_objects_quantity();
         if(must_wait){
+            /* The second condition stands for that as long as there are objects finalized the current thread will continue waiting */
             if((status != TM_ERROR_NONE) && (fin_obj_num == temp)) break;
         }else{
             if(status != TM_ERROR_NONE) break; 
@@ -216,24 +203,22 @@
     JavaVM *java_vm = (JavaVM *)args[0];
     JNIEnv *jni_env;
     char *name = "finalizer";
-    // FIXME: use args[1] (thread number) to distinguish finalization threads by name
 
     JavaVMAttachArgs *jni_args = (JavaVMAttachArgs*)STD_MALLOC(sizeof(JavaVMAttachArgs));
     jni_args->version = JNI_VERSION_1_2;
     jni_args->name = name;
-    jni_args->group = (jobject)args[2];
+    jni_args->group = (jobject)args[1];
     IDATA status = AttachCurrentThreadAsDaemon(java_vm, (void**)&jni_env, jni_args);
     assert(status == JNI_OK);
     set_current_thread_context_loader(jni_env);
-    assert(!get_fin_thread_attached());
-    set_fin_thread_attached();
+    hysem_post(fin_thread_info->attached_sem);
     
     /* Choice: use VM_thread or hythread to indicate the finalizer thread ?
      * Now we use hythread
      * p_TLS_vmthread->finalize_thread_flags = thread_id;
      */
     
-    while(true){
+    while(TRUE){
         /* Waiting for pending finalizers */
         wait_pending_finalizer();
         
@@ -254,8 +239,7 @@
     vm_heavy_finalizer_resume_mutator();
     status = DetachCurrentThread(java_vm);
     assert(status == JNI_OK);
-    dec_fin_thread_num();
-    //status = jthread_detach(java_thread);
+    atomic_dec32(&fin_thread_info->thread_num);
     return status;
 }
 
@@ -279,7 +263,7 @@
 
 void vm_heavy_finalizer_block_mutator(void)
 {
-    /* Maybe self test is not needed. This needs further test. */
+    /* Finalizer thread shouldn't block itself. Maybe self test is not needed. This needs further test. */
     if(self_is_finalizer_thread())
         return;
     
@@ -297,11 +281,4 @@
     if(gc_clear_mutator_block_flag())
         hycond_notify_all(&fin_thread_info->mutator_block_cond);
 }
-
-
-
-
-
-
-
 

Modified: harmony/enhanced/drlvm/trunk/vm/vmcore/src/init/ref_enqueue_thread.cpp
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/src/init/ref_enqueue_thread.cpp?view=diff&rev=561762&r1=561761&r2=561762
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/vmcore/src/init/ref_enqueue_thread.cpp (original)
+++ harmony/enhanced/drlvm/trunk/vm/vmcore/src/init/ref_enqueue_thread.cpp Wed Aug  1 04:11:15 2007
@@ -38,9 +38,8 @@
 {  native_ref_thread_flag = flag; }
 
 
+extern jobject get_system_thread_group(JNIEnv *jni_env);
 static IDATA ref_enqueue_thread_func(void **args);
-static void wait_ref_thread_attached(void);
-extern jobject get_system_thread_group(JNIEnv* jni_env);
 
 void ref_enqueue_thread_init(JavaVM *java_vm, JNIEnv* jni_env)
 {
@@ -49,23 +48,27 @@
     
     ref_thread_info = (Ref_Enqueue_Thread_Info *)STD_MALLOC(sizeof(Ref_Enqueue_Thread_Info));
     ref_thread_info->shutdown = false;
-    ref_thread_info->thread_attached = 0;
     
     IDATA status = hysem_create(&ref_thread_info->pending_sem, 0, REF_ENQUEUE_THREAD_NUM);
     assert(status == TM_ERROR_NONE);
     
+    status = hysem_create(&ref_thread_info->attached_sem, 0, REF_ENQUEUE_THREAD_NUM);
+    assert(status == TM_ERROR_NONE);
+    
     status = hycond_create(&ref_thread_info->end_cond);
     assert(status == TM_ERROR_NONE);
     status = hymutex_create(&ref_thread_info->end_mutex, TM_MUTEX_DEFAULT);
     assert(status == TM_ERROR_NONE);
     
+    ref_thread_info->thread_num = REF_ENQUEUE_THREAD_NUM;
+    
     void **args = (void **)STD_MALLOC(sizeof(void *)*2);
     args[0] = (void *)java_vm;
     args[1] = (void*)get_system_thread_group(jni_env);
     status = hythread_create(NULL, 0, REF_ENQUEUE_THREAD_PRIORITY, 0, (hythread_entrypoint_t)ref_enqueue_thread_func, args);
     assert(status == TM_ERROR_NONE);
     
-    wait_ref_thread_attached();
+    hysem_wait(ref_thread_info->attached_sem);
 }
 
 void ref_enqueue_shutdown(void)
@@ -80,17 +83,17 @@
 static uint32 atomic_dec32(volatile apr_uint32_t *mem)
 {  return (uint32)apr_atomic_dec32(mem); }
 
-static void inc_ref_thread_num(void)
-{ atomic_inc32(&ref_thread_info->thread_attached); }
-
-static void dec_ref_thread_num(void)
-{ atomic_dec32(&ref_thread_info->thread_attached); }
-
-static void wait_ref_thread_attached(void)
-{ while(ref_thread_info->thread_attached < REF_ENQUEUE_THREAD_NUM); }
-
 void wait_native_ref_thread_detached(void)
-{ while(ref_thread_info->thread_attached){hythread_yield();} }
+{
+    hymutex_lock(&ref_thread_info->end_mutex);
+    while(ref_thread_info->thread_num){
+        atomic_inc32(&ref_thread_info->end_waiting_num);
+        IDATA status = hycond_wait_timed(&ref_thread_info->end_cond, &ref_thread_info->end_mutex, (I_64)1000, 0);
+        atomic_dec32(&ref_thread_info->end_waiting_num);
+        if(status != TM_ERROR_NONE) break;
+    }
+    hymutex_unlock(&ref_thread_info->end_mutex);
+}
 
 static void wait_ref_enqueue_end(void)
 {
@@ -109,7 +112,7 @@
 
 void activate_ref_enqueue_thread(Boolean wait)
 {
-    IDATA stat = hysem_set(ref_thread_info->pending_sem, REF_ENQUEUE_THREAD_NUM);
+    IDATA stat = hysem_set(ref_thread_info->pending_sem, ref_thread_info->thread_num);
     assert(stat == TM_ERROR_NONE);
     
     if(wait)
@@ -143,9 +146,9 @@
     IDATA status = AttachCurrentThreadAsDaemon(java_vm, (void**)&jni_env, jni_args);
     assert(status == JNI_OK);
     set_current_thread_context_loader(jni_env);
-    inc_ref_thread_num();
+    hysem_post(ref_thread_info->attached_sem);
     
-    while(true){
+    while(TRUE){
         /* Waiting for pending weak references */
         wait_pending_reference();
         
@@ -160,7 +163,6 @@
     }
     
     status = DetachCurrentThread(java_vm);
-    dec_ref_thread_num();
-    //status = jthread_detach(java_thread);
+    atomic_dec32(&ref_thread_info->thread_num);
     return status;
 }