Author: labath Date: Wed Sep 23 05:16:57 2015 New Revision: 248371 URL: http://llvm.org/viewvc/llvm-project?rev=248371&view=rev Log: Fix race condition during process detach
Summary: 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. Reviewers: jingham, clayborg Subscribers: lldb-commits Differential Revision: http://reviews.llvm.org/D13056 Modified: lldb/trunk/include/lldb/Target/Process.h lldb/trunk/source/Target/Process.cpp lldb/trunk/test/functionalities/attach_resume/TestAttachResume.py Modified: lldb/trunk/include/lldb/Target/Process.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=248371&r1=248370&r2=248371&view=diff ============================================================================== --- lldb/trunk/include/lldb/Target/Process.h (original) +++ lldb/trunk/include/lldb/Target/Process.h Wed Sep 23 05:16:57 2015 @@ -3479,7 +3479,7 @@ protected: } Error - HaltForDestroyOrDetach(lldb::EventSP &exit_event_sp); + StopForDestroyOrDetach(lldb::EventSP &exit_event_sp); bool StateChangedIsExternallyHijacked(); Modified: lldb/trunk/source/Target/Process.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=248371&r1=248370&r2=248371&view=diff ============================================================================== --- lldb/trunk/source/Target/Process.cpp (original) +++ lldb/trunk/source/Target/Process.cpp Wed Sep 23 05:16:57 2015 @@ -3916,52 +3916,46 @@ Process::Halt (bool clear_thread_plans) } 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 @@ Process::Detach (bool keep_stopped) { if (DetachRequiresHalt()) { - error = HaltForDestroyOrDetach (exit_event_sp); + error = StopForDestroyOrDetach (exit_event_sp); if (!error.Success()) { m_destroy_in_process = false; @@ -4054,7 +4048,7 @@ Process::Destroy (bool force_kill) 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 @@ public: break; case 'i': if (StateIsRunningState(m_process->GetState())) - m_process->Halt(); + m_process->SendAsyncInterrupt(); break; } } @@ -5256,8 +5250,8 @@ public: // 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 Modified: lldb/trunk/test/functionalities/attach_resume/TestAttachResume.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/attach_resume/TestAttachResume.py?rev=248371&r1=248370&r2=248371&view=diff ============================================================================== --- lldb/trunk/test/functionalities/attach_resume/TestAttachResume.py (original) +++ lldb/trunk/test/functionalities/attach_resume/TestAttachResume.py Wed Sep 23 05:16:57 2015 @@ -15,7 +15,6 @@ class AttachResumeTestCase(TestBase): mydir = TestBase.compute_mydir(__file__) @expectedFailureFreeBSD('llvm.org/pr19310') - @expectedFlakeyLinux('llvm.org/pr19310') @expectedFailureWindows("llvm.org/pr24778") @skipIfRemote @dwarf_test _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
