From c8c77d46efbe1d33a07a425bd84d82eae30bd959 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Mon, 28 Sep 2015 09:37:51 +0000 Subject: [PATCH] Revert "Fix race condition during process detach" This fix is not correct on its own until D12968 is resolved. Will resumbit once that is done. llvm-svn: 248702 --- lldb/include/lldb/Target/Process.h | 2 +- lldb/source/Target/Process.cpp | 76 ++++++++++--------- .../attach_resume/TestAttachResume.py | 1 + 3 files changed, 43 insertions(+), 36 deletions(-) diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 26609cc41055..90eb372a28e6 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -3479,7 +3479,7 @@ protected: } Error - StopForDestroyOrDetach(lldb::EventSP &exit_event_sp); + HaltForDestroyOrDetach(lldb::EventSP &exit_event_sp); bool StateChangedIsExternallyHijacked(); diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 39d29987f09d..a8f43c36a048 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -3916,46 +3916,52 @@ Process::Halt (bool clear_thread_plans) } Error -Process::StopForDestroyOrDetach(lldb::EventSP &exit_event_sp) +Process::HaltForDestroyOrDetach(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 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) + log->Printf("Process::%s() About to halt.", __FUNCTION__); + error = Halt(); + if (error.Success()) { - 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::%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) + // 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; + } + } + } + else + { + if (log) + log->Printf("Process::HaltForDestroyOrDetach() Halt got error: %s", error.AsCString()); } } return error; @@ -3974,7 +3980,7 @@ Process::Detach (bool keep_stopped) { if (DetachRequiresHalt()) { - error = StopForDestroyOrDetach (exit_event_sp); + error = HaltForDestroyOrDetach (exit_event_sp); if (!error.Success()) { m_destroy_in_process = false; @@ -4048,7 +4054,7 @@ Process::Destroy (bool force_kill) EventSP exit_event_sp; if (DestroyRequiresHalt()) { - error = StopForDestroyOrDetach(exit_event_sp); + error = HaltForDestroyOrDetach(exit_event_sp); } if (m_public_state.GetValue() != eStateRunning) @@ -5225,7 +5231,7 @@ public: break; case 'i': if (StateIsRunningState(m_process->GetState())) - m_process->SendAsyncInterrupt(); + m_process->Halt(); break; } } @@ -5250,8 +5256,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->SendAsyncInterrupt() - // from a much safer location in code. + // that was written to the pipe and then call m_process->Halt() from a + // much safer location in code. if (m_active) { char ch = 'i'; // Send 'i' for interrupt diff --git a/lldb/test/functionalities/attach_resume/TestAttachResume.py b/lldb/test/functionalities/attach_resume/TestAttachResume.py index 6fcefa43cddb..ec6e626ac1ca 100644 --- a/lldb/test/functionalities/attach_resume/TestAttachResume.py +++ b/lldb/test/functionalities/attach_resume/TestAttachResume.py @@ -15,6 +15,7 @@ class AttachResumeTestCase(TestBase): mydir = TestBase.compute_mydir(__file__) @expectedFailureFreeBSD('llvm.org/pr19310') + @expectedFlakeyLinux('llvm.org/pr19310') @expectedFailureWindows("llvm.org/pr24778") @skipIfRemote @dwarf_test