labath created this revision.
labath added reviewers: jingham, clayborg.
labath added a subscriber: lldb-commits.

The following situation occured in TestAttachResume:

The inferior was stoped at a breakpoint and we did a continue, immediately 
followed by a detach.
Since there was a trap instruction under the IP, the continue did a 
step-over-breakpoint before
resuming the inferior for real. In some cases, the detach command was executed 
between these two
events (after the step-over stop, but before continue). Here, public state was 
running, but
private state was stopped. This caused a problem because HaltForDestroyOrDetach 
was checking the
public state to see whether it needs to stop the process (call Halt()), but 
Halt() was checking
the private state and concluded that there is nothing for it to do.

Solution: Instead of Halt() call SendAsyncInterrupt(), which will then cause 
Halt() to be
executed in the context of the private state thread. I also rename 
HaltForDestroyOrDetach to
reflect it does not call halt directly.

http://reviews.llvm.org/D13056

Files:
  include/lldb/Target/Process.h
  source/Target/Process.cpp
  test/functionalities/attach_resume/TestAttachResume.py
  test/lang/go/goroutines/a.out
  test/lang/go/types/a.out

Index: test/functionalities/attach_resume/TestAttachResume.py
===================================================================
--- test/functionalities/attach_resume/TestAttachResume.py
+++ test/functionalities/attach_resume/TestAttachResume.py
@@ -15,7 +15,6 @@
     mydir = TestBase.compute_mydir(__file__)
 
     @expectedFailureFreeBSD('llvm.org/pr19310')
-    @expectedFlakeyLinux('llvm.org/pr19310')
     @expectedFailureWindows("llvm.org/pr24778")
     @skipIfRemote
     @dwarf_test
Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -3916,52 +3916,46 @@
 }
 
 Error
-Process::HaltForDestroyOrDetach(lldb::EventSP &exit_event_sp)
+Process::StopForDestroyOrDetach(lldb::EventSP &exit_event_sp)
 {
     Error error;
     if (m_public_state.GetValue() == eStateRunning)
     {
         Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
         if (log)
-            log->Printf("Process::%s() About to halt.", __FUNCTION__);
-        error = Halt();
-        if (error.Success())
+            log->Printf("Process::%s() About to stop.", __FUNCTION__);
+
+        SendAsyncInterrupt();
+
+        // Consume the interrupt event.
+        TimeValue timeout (TimeValue::Now());
+        timeout.OffsetWithSeconds(10);
+        StateType state = WaitForProcessToStop (&timeout, &exit_event_sp);
+
+        // If the process exited while we were waiting for it to stop, put the exited event into
+        // the shared pointer passed in and return.  Our caller doesn't need to do anything else, since
+        // they don't have a process anymore...
+
+        if (state == eStateExited || m_private_state.GetValue() == eStateExited)
         {
-            // Consume the halt event.
-            TimeValue timeout (TimeValue::Now());
-            timeout.OffsetWithSeconds(1);
-            StateType state = WaitForProcessToStop (&timeout, &exit_event_sp);
-            
-            // If the process exited while we were waiting for it to stop, put the exited event into
-            // the shared pointer passed in and return.  Our caller doesn't need to do anything else, since
-            // they don't have a process anymore...
-            
-            if (state == eStateExited || m_private_state.GetValue() == eStateExited)
-            {
-                if (log)
-                    log->Printf("Process::HaltForDestroyOrDetach() Process exited while waiting to Halt.");
-                return error;
-            }
-            else
-                exit_event_sp.reset(); // It is ok to consume any non-exit stop events
-    
-            if (state != eStateStopped)
-            {
-                if (log)
-                    log->Printf("Process::HaltForDestroyOrDetach() Halt failed to stop, state is: %s", StateAsCString(state));
-                // If we really couldn't stop the process then we should just error out here, but if the
-                // lower levels just bobbled sending the event and we really are stopped, then continue on.
-                StateType private_state = m_private_state.GetValue();
-                if (private_state != eStateStopped)
-                {
-                    return error;
-                }
-            }
+            if (log)
+                log->Printf("Process::%s() Process exited while waiting to stop.", __FUNCTION__);
+            return error;
         }
         else
+            exit_event_sp.reset(); // It is ok to consume any non-exit stop events
+
+        if (state != eStateStopped)
         {
             if (log)
-                log->Printf("Process::HaltForDestroyOrDetach() Halt got error: %s", error.AsCString());
+                log->Printf("Process::%s() failed to stop, state is: %s", __FUNCTION__, StateAsCString(state));
+            // If we really couldn't stop the process then we should just error out here, but if the
+            // lower levels just bobbled sending the event and we really are stopped, then continue on.
+            StateType private_state = m_private_state.GetValue();
+            if (private_state != eStateStopped)
+            {
+                return error;
+            }
         }
     }
     return error;
@@ -3980,7 +3974,7 @@
     {
         if (DetachRequiresHalt())
         {
-            error = HaltForDestroyOrDetach (exit_event_sp);
+            error = StopForDestroyOrDetach (exit_event_sp);
             if (!error.Success())
             {
                 m_destroy_in_process = false;
@@ -4054,7 +4048,7 @@
         EventSP exit_event_sp;
         if (DestroyRequiresHalt())
         {
-            error = HaltForDestroyOrDetach(exit_event_sp);
+            error = StopForDestroyOrDetach(exit_event_sp);
         }
         
         if (m_public_state.GetValue() != eStateRunning)
@@ -5231,7 +5225,7 @@
                                 break;
                             case 'i':
                                 if (StateIsRunningState(m_process->GetState()))
-                                    m_process->Halt();
+                                    m_process->SendAsyncInterrupt();
                                 break;
                         }
                     }
@@ -5256,8 +5250,8 @@
         // Do only things that are safe to do in an interrupt context (like in
         // a SIGINT handler), like write 1 byte to a file descriptor. This will
         // interrupt the IOHandlerProcessSTDIO::Run() and we can look at the byte
-        // that was written to the pipe and then call m_process->Halt() from a
-        // much safer location in code.
+        // that was written to the pipe and then call m_process->SendAsyncInterrupt()
+        // from a much safer location in code.
         if (m_active)
         {
             char ch = 'i'; // Send 'i' for interrupt
Index: include/lldb/Target/Process.h
===================================================================
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -3479,7 +3479,7 @@
     }
     
     Error
-    HaltForDestroyOrDetach(lldb::EventSP &exit_event_sp);
+    StopForDestroyOrDetach(lldb::EventSP &exit_event_sp);
 
     bool
     StateChangedIsExternallyHijacked();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to