[Lldb-commits] [lldb] r248371 - Fix race condition during process detach
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 23 03:17:00 PDT 2015
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
More information about the lldb-commits
mailing list