[Lldb-commits] [PATCH] D68174: Allow private-state-thread to call SB API's that take the TargetAPI mutex
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL373280: Allow the internal-state-thread free access to the TargetAPI mutex. (authored by jingham, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D68174?vs=74=222527#toc Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68174/new/ https://reviews.llvm.org/D68174 Files: lldb/trunk/include/lldb/Target/Process.h lldb/trunk/include/lldb/Target/Target.h lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c lldb/trunk/source/Target/Process.cpp lldb/trunk/source/Target/Target.cpp Index: lldb/trunk/source/Target/Target.cpp === --- lldb/trunk/source/Target/Target.cpp +++ lldb/trunk/source/Target/Target.cpp @@ -4129,3 +4129,10 @@ module_list = event_data->m_module_list; return module_list; } + +std::recursive_mutex ::GetAPIMutex() { + if (GetProcessSP() && GetProcessSP()->CurrentThreadIsPrivateStateThread()) +return m_private_mutex; + else +return m_mutex; +} Index: lldb/trunk/source/Target/Process.cpp === --- lldb/trunk/source/Target/Process.cpp +++ lldb/trunk/source/Target/Process.cpp @@ -5538,6 +5538,12 @@ return m_public_run_lock; } +bool Process::CurrentThreadIsPrivateStateThread() +{ + return m_private_state_thread.EqualsThread(Host::GetCurrentThread()); +} + + void Process::Flush() { m_thread_list.Flush(); m_extended_thread_list.Flush(); Index: lldb/trunk/include/lldb/Target/Target.h === --- lldb/trunk/include/lldb/Target/Target.h +++ lldb/trunk/include/lldb/Target/Target.h @@ -535,7 +535,7 @@ static const lldb::TargetPropertiesSP (); - std::recursive_mutex () { return m_mutex; } + std::recursive_mutex (); void DeleteCurrentProcess(); @@ -1288,6 +1288,12 @@ lldb::PlatformSP m_platform_sp; ///< The platform for this target. std::recursive_mutex m_mutex; ///< An API mutex that is used by the lldb::SB* /// classes make the SB interface thread safe + /// When the private state thread calls SB API's - usually because it is + /// running OS plugin or Python ThreadPlan code - it should not block on the + /// API mutex that is held by the code that kicked off the sequence of events + /// that led us to run the code. We hand out this mutex instead when we + /// detect that code is running on the private state thread. + std::recursive_mutex m_private_mutex; Arch m_arch; ModuleList m_images; ///< The list of images for this process (shared /// libraries and anything dynamically loaded). Index: lldb/trunk/include/lldb/Target/Process.h === --- lldb/trunk/include/lldb/Target/Process.h +++ lldb/trunk/include/lldb/Target/Process.h @@ -2272,6 +2272,8 @@ void ClearPreResumeAction(PreResumeActionCallback callback, void *baton); ProcessRunLock (); + + bool CurrentThreadIsPrivateStateThread(); virtual Status SendEventData(const char *data) { Status return_error("Sending an event is not supported for this process."); Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c === --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c @@ -1,7 +1,10 @@ #include void foo() { - printf("Set a breakpoint here.\n"); + int foo = 10; + printf("%d\n", foo); // Set a breakpoint here. + foo = 20; + printf("%d\n", foo); } int main() { Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py === --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py @@ -35,3 +35,38 @@ def queue_child_thread_plan(self): return self.thread_plan.QueueThreadPlanForStepScripted("Steps.StepOut") + +# This plan does a step-over until a variable changes value. +class StepUntil(StepWithChild): +def __init__(self, thread_plan, dict): +self.frame = thread_plan.GetThread().frames[0] +self.target = thread_plan.GetThread().GetProcess().GetTarget() +self.value = self.frame.FindVariable("foo") +
[Lldb-commits] [PATCH] D68174: Allow private-state-thread to call SB API's that take the TargetAPI mutex
jingham added a comment. Note, a better way to do this would be to have some kind of "API/Runlock baton" that could get handed off from the thread that initiated the continue to the private state thread, then handed back when the process stops and returns control to the SB API that initiated the run. But I don't have time to design such a thing right now, and this was blocking writing tests for some work I am doing to handle the case where OS Plugins don't report all threads at each stop. So I'd prefer this more straightforward solution for now - especially since it's just a copy of the way we handled the same problem for the runlock. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68174/new/ https://reviews.llvm.org/D68174 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68174: Allow private-state-thread to call SB API's that take the TargetAPI mutex
jingham created this revision. jingham added a reviewer: clayborg. Herald added subscribers: lldb-commits, JDevlieghere. Herald added a project: LLDB. When we do synchronous process execution in lldb, the call that restarted the target will generally have acquired first the TargetAPI lock, and then the Process RunLock. If in the process of handling that continue we need to run Python code in the private-state-thread (e.g. for an OS plugin or scripted ThreadPlan) if that Python code calls another API that takes the Target API mutex, we will deadlock. However, the private state thread is always doing work on behalf of whatever thread initiated the continue. So the private state thread should always formally be allowed to pass the lock check. We already handled this in the case of the Process RunLock by passing out separate locks depending on whether you were running code in the private state thread or anywhere else. This patch just does the same thing for the Target API mutex. I also added a test using a scripted ThreadPlan that calls various SBFrame and SBValue API's which take the API lock in the should_stop method of the plan (which is always run on the private state thread). Repository: rLLDB LLDB https://reviews.llvm.org/D68174 Files: lldb/include/lldb/Target/Process.h lldb/include/lldb/Target/Target.h lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c lldb/source/Target/Process.cpp lldb/source/Target/Target.cpp Index: lldb/source/Target/Target.cpp === --- lldb/source/Target/Target.cpp +++ lldb/source/Target/Target.cpp @@ -4129,3 +4129,10 @@ module_list = event_data->m_module_list; return module_list; } + +std::recursive_mutex ::GetAPIMutex() { + if (GetProcessSP() && GetProcessSP()->CurrentThreadIsPrivateStateThread()) +return m_private_mutex; + else +return m_mutex; +} Index: lldb/source/Target/Process.cpp === --- lldb/source/Target/Process.cpp +++ lldb/source/Target/Process.cpp @@ -5538,6 +5538,12 @@ return m_public_run_lock; } +bool Process::CurrentThreadIsPrivateStateThread() +{ + return m_private_state_thread.EqualsThread(Host::GetCurrentThread()); +} + + void Process::Flush() { m_thread_list.Flush(); m_extended_thread_list.Flush(); Index: lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c === --- lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c +++ lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c @@ -1,7 +1,10 @@ #include void foo() { - printf("Set a breakpoint here.\n"); + int foo = 10; + printf("%d\n", foo); // Set a breakpoint here. + foo = 20; + printf("%d\n", foo); } int main() { Index: lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py === --- lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py +++ lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py @@ -12,23 +12,23 @@ NO_DEBUG_INFO_TESTCASE = True +def setUp(self): +TestBase.setUp(self) +self.main_source_file = lldb.SBFileSpec("main.c") +self.runCmd("command script import Steps.py") + def test_standard_step_out(self): -"""Tests stepping with the scripted thread plan laying over a standard thread plan for stepping out.""" +"""Tests stepping with the scripted thread plan laying over a standard +thread plan for stepping out.""" self.build() -self.main_source_file = lldb.SBFileSpec("main.c") self.step_out_with_scripted_plan("Steps.StepOut") def test_scripted_step_out(self): -"""Tests stepping with the scripted thread plan laying over an another scripted thread plan for stepping out.""" +"""Tests stepping with the scripted thread plan laying over an another +scripted thread plan for stepping out.""" self.build() -self.main_source_file = lldb.SBFileSpec("main.c") self.step_out_with_scripted_plan("Steps.StepScripted") -def setUp(self): -TestBase.setUp(self) -self.main_source_file = lldb.SBFileSpec("main.c") -self.runCmd("command script import Steps.py") - def step_out_with_scripted_plan(self, name): (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, "Set a breakpoint here", @@ -43,6 +43,7 @@ frame = thread.GetFrameAtIndex(0)