You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by ge...@apache.org on 2006/10/06 18:49:01 UTC

svn commit: r453673 - in /incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src: exception/exceptions_jit.cpp jit/ini.cpp jvmti/jvmti_break.cpp jvmti/jvmti_event.cpp jvmti/jvmti_pop_frame.cpp jvmti/jvmti_step.cpp

Author: geirm
Date: Fri Oct  6 09:48:59 2006
New Revision: 453673

URL: http://svn.apache.org/viewvc?view=rev&rev=453673
Log:
HARMONY-1706

Here is a synchronization problem during processing Single Step breakpoints. One thread gets lock and tries to 
suspend other threads, while the second thread is not in safe region, tries to get the same lock and cannot reach 
safe point. So, there is a dead lock.

Ubuntu 6 , smoke, c-unit, ~kernel


Modified:
    incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/exception/exceptions_jit.cpp
    incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jit/ini.cpp
    incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_break.cpp
    incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_event.cpp
    incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_pop_frame.cpp
    incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_step.cpp

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/exception/exceptions_jit.cpp
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/exception/exceptions_jit.cpp?view=diff&rev=453673&r1=453672&r2=453673
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/exception/exceptions_jit.cpp (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/exception/exceptions_jit.cpp Fri Oct  6 09:48:59 2006
@@ -222,9 +222,9 @@
     if (ti->isEnabled() && ti->is_single_step_enabled())
     {
         VM_thread *vm_thread = p_TLS_vmthread;
+        LMAutoUnlock lock(ti->vm_brpt->get_lock());
         if (NULL != vm_thread->ss_state)
         {
-            LMAutoUnlock lock(ti->vm_brpt->get_lock());
             jvmti_remove_single_step_breakpoints(ti, vm_thread);
         }
     }
@@ -269,10 +269,9 @@
                     ti->getPhase() == JVMTI_PHASE_LIVE)
                 {
                     VM_thread *vm_thread = p_TLS_vmthread;
+                    LMAutoUnlock lock(ti->vm_brpt->get_lock());
                     if (NULL != vm_thread->ss_state)
                     {
-                        LMAutoUnlock lock(ti->vm_brpt->get_lock());
-
                         uint16 bc;
                         NativeCodePtr ip = handler->get_handler_ip();
                         OpenExeJpdaError UNREF result =

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jit/ini.cpp
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jit/ini.cpp?view=diff&rev=453673&r1=453672&r2=453673
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jit/ini.cpp (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jit/ini.cpp Fri Oct  6 09:48:59 2006
@@ -44,10 +44,10 @@
         ti->getPhase() == JVMTI_PHASE_LIVE)
     {
         VM_thread *vm_thread = p_TLS_vmthread;
+        LMAutoUnlock lock(ti->vm_brpt->get_lock());
         if (NULL != vm_thread->ss_state)
         {
             // Start single stepping a new Java method
-            LMAutoUnlock lock(ti->vm_brpt->get_lock());
             jvmti_remove_single_step_breakpoints(ti, vm_thread);
 
             jvmti_StepLocation method_start = {(Method *)method, 0};
@@ -64,10 +64,9 @@
         ti->getPhase() == JVMTI_PHASE_LIVE)
     {
         VM_thread *vm_thread = p_TLS_vmthread;
+        LMAutoUnlock lock(ti->vm_brpt->get_lock());
         if (NULL != vm_thread->ss_state)
         {
-            LMAutoUnlock lock(ti->vm_brpt->get_lock());
-
             jvmti_StepLocation *method_return;
             unsigned locations_number;
 

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_break.cpp
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_break.cpp?view=diff&rev=453673&r1=453672&r2=453673
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_break.cpp (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_break.cpp Fri Oct  6 09:48:59 2006
@@ -117,8 +117,8 @@
         }
     }
 
-    tmn_suspend_disable();
     oh_discard_local_handle(hThread);
+    tmn_suspend_disable();
 
     return true;
 }

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_event.cpp
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_event.cpp?view=diff&rev=453673&r1=453672&r2=453673
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_event.cpp (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_event.cpp Fri Oct  6 09:48:59 2006
@@ -1677,6 +1677,7 @@
         {
             // Init single step state for the thread
             VM_thread *vm_thread = p_TLS_vmthread;
+            LMAutoUnlock lock(ti->vm_brpt->get_lock());
 
             jvmtiError UNREF errorCode = _allocate(sizeof(JVMTISingleStepState),
                 (unsigned char **)&vm_thread->ss_state);

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_pop_frame.cpp
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_pop_frame.cpp?view=diff&rev=453673&r1=453672&r2=453673
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_pop_frame.cpp (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_pop_frame.cpp Fri Oct  6 09:48:59 2006
@@ -328,9 +328,9 @@
     if (ti->isEnabled() && ti->is_single_step_enabled())
     {
         VM_thread *vm_thread = p_TLS_vmthread;
+        LMAutoUnlock lock(ti->vm_brpt->get_lock());
         if (NULL != vm_thread->ss_state) {
             // remove old single step breakpoints
-            LMAutoUnlock lock(ti->vm_brpt->get_lock());
             jvmti_remove_single_step_breakpoints(ti, vm_thread);
 
             // set new single step breakpoints

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_step.cpp
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_step.cpp?view=diff&rev=453673&r1=453672&r2=453673
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_step.cpp (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_step.cpp Fri Oct  6 09:48:59 2006
@@ -441,6 +441,10 @@
     jvmti_SingleStepLocation(vm_thread, m, (unsigned)location,
                             &locations, &locations_count);
 
+    // lock breakpoints
+    VMBreakPoints* vm_brpt = VM_Global_State::loader_env->TI->vm_brpt;
+    LMAutoUnlock lock(vm_brpt->get_lock());
+
     jvmti_remove_single_step_breakpoints(ti, vm_thread);
 
     jvmti_set_single_step_breakpoints(ti, vm_thread, locations, locations_count);
@@ -507,6 +511,11 @@
     }
 
     TRACE2("jvmti.break.ss", "Removing VIRTUAL single step breakpoint: " << bp->addr);
+
+    // lock breakpoints
+    VMBreakPoints* vm_brpt = VM_Global_State::loader_env->TI->vm_brpt;
+    LMAutoUnlock lock(vm_brpt->get_lock());
+
     // The determined method is the one which is called by
     // invokevirtual or invokeinterface bytecodes. It should be
     // started to be single stepped from the beginning
@@ -532,10 +541,14 @@
     if (!ti->isEnabled() || ti->getPhase() != JVMTI_PHASE_LIVE)
         return false;
 
+    ti->vm_brpt->lock();
     JVMTISingleStepState* sss = p_TLS_vmthread->ss_state;
 
-    if (!sss || !ti->is_single_step_enabled())
+    if (!sss || !ti->is_single_step_enabled()) {
+        ti->vm_brpt->unlock();
         return false;
+    }
+    ti->vm_brpt->unlock();
 
     jlocation location = bp->location;
     jmethodID method = bp->method;
@@ -629,8 +642,8 @@
     if (ti->is_single_step_enabled())
         jvmti_setup_jit_single_step(ti, m, location);
 
-    tmn_suspend_disable();
     oh_discard_local_handle(hThread);
+    tmn_suspend_disable();
 
     return true;
 }
@@ -642,6 +655,10 @@
     ASSERT_NO_INTERPRETER;
 
     JVMTISingleStepState *ss_state = vm_thread->ss_state;
+    if( NULL == ss_state ) {
+        // no need predict next step due to single step is off
+        return;
+    }
 
     if (NULL == ss_state->predicted_breakpoints)
     {
@@ -691,8 +708,11 @@
 
     TRACE2("jvmti.break.ss", "Remove single step breakpoints");
 
-    if (ss_state && ss_state->predicted_breakpoints)
+    if (ss_state && ss_state->predicted_breakpoints) {
+        TRACE2("jvmti.break.ss", "Remove single step, intf: "
+            << ss_state->predicted_breakpoints);
         ss_state->predicted_breakpoints->remove_all_reference();
+    }
 }
 
 jvmtiError jvmti_get_next_bytecodes_from_native(VM_thread *thread,
@@ -857,16 +877,27 @@
 {
     assert(hythread_is_suspend_enabled());
 
-    VMBreakPoints* vm_brpt = VM_Global_State::loader_env->TI->vm_brpt;
-    LMAutoUnlock lock(vm_brpt->get_lock());
-
     hythread_iterator_t threads_iterator;
 
+    if( single_step_enabled ) {
+        // single step is already enabled
+        return JVMTI_ERROR_NONE;
+    }
+
     // Suspend all threads except current
     IDATA tm_ret = hythread_suspend_all(&threads_iterator, NULL);
     if (TM_ERROR_NONE != tm_ret)
         return JVMTI_ERROR_INTERNAL;
 
+    // get this lock for insurance - all threads are suspended
+    VMBreakPoints* vm_brpt = VM_Global_State::loader_env->TI->vm_brpt;
+    LMAutoUnlock lock(vm_brpt->get_lock());
+
+    if( single_step_enabled ) {
+        // single step is already enabled
+        return JVMTI_ERROR_NONE;
+    }
+
     hythread_t ht;
 
     // Set single step in all threads
@@ -920,16 +951,26 @@
 {
     assert(hythread_is_suspend_enabled());
 
-    VMBreakPoints* vm_brpt = VM_Global_State::loader_env->TI->vm_brpt;
-    LMAutoUnlock lock(vm_brpt->get_lock());
-
     hythread_iterator_t threads_iterator;
 
+    if( !single_step_enabled ) {
+        // single step is already disabled
+        return JVMTI_ERROR_NONE;
+    }
     // Suspend all threads except current
     IDATA tm_ret = hythread_suspend_all(&threads_iterator, NULL);
     if (TM_ERROR_NONE != tm_ret)
         return JVMTI_ERROR_INTERNAL;
 
+    // get this lock for insurance - all threads are suspended
+    VMBreakPoints* vm_brpt = VM_Global_State::loader_env->TI->vm_brpt;
+    LMAutoUnlock lock(vm_brpt->get_lock());
+
+    if( !single_step_enabled ) {
+        // single step is already disabled
+        return JVMTI_ERROR_NONE;
+    }
+
     hythread_t ht;
 
     // Clear single step in all threads
@@ -942,10 +983,12 @@
             continue;
         }
 
-        jvmti_remove_single_step_breakpoints(this, vm_thread);
-        vm_brpt->release_intf(vm_thread->ss_state->predicted_breakpoints);
-        _deallocate((unsigned char *)vm_thread->ss_state);
-        vm_thread->ss_state = NULL;
+        if( vm_thread->ss_state ) {
+            jvmti_remove_single_step_breakpoints(this, vm_thread);
+            vm_brpt->release_intf(vm_thread->ss_state->predicted_breakpoints);
+            _deallocate((unsigned char *)vm_thread->ss_state);
+            vm_thread->ss_state = NULL;
+        }
     }
 
     single_step_enabled = false;