[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

[Ilya Bukonkin via Phabricator via lldb-commits]

fallkrum updated this revision to Diff 269067.
fallkrum added a comment.

In D80112#2075097 <https://reviews.llvm.org/D80112#2075097>, @jingham wrote:

> If I understand the problem you are describing, it is that you suspended a thread as a user-level suspend, so it's stop reason got stuck.  That seems okay, its probably useful to keep the stop reason of the thread at what it was when you suspended it.  But that means  when gets asked to do its action again, the action is stale and does the wrong thing.  If that's what's going wrong,


Yes, you understood right, that's exactly what was going wrong.

> Actually, you can more easily do that. The thread iteration where we call PerformAction etc. works on a copy of the thread list (since one thread's action could cause the thread list to change). So if you just change the copy operation to only copy over threads which aren't user-suspended, then you should be set.

I don't see that's a copy, it seems that's a reference on the actual thread list:

  ThreadList &curr_thread_list = process_sp->GetThreadList();

So I decided to filter out all suspended thread as you recommended.

> It would be good to write a end-to-end test that mirrors this behavior.  It would be pretty straightforward to write an API test that runs a process, sets a breakpoint, hits it on one thread, suspends that thread, then hits it on another thread and make sure the action didn't run twice on the second stop.

Don't see it is possible to write such a test due to StopInfoBreakpoint::PerformAction implementation. This method executes only once and saves results of execution so the next time we call PerformAction it will not execute breakpoint's  callback and we have no chance to catch action. Wrote unit tests for ProcessEventData that mirror behaviour.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80112/new/

https://reviews.llvm.org/D80112

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Target/Process.cpp
  lldb/unittests/CMakeLists.txt
  lldb/unittests/Process/CMakeLists.txt
  lldb/unittests/Process/ProcessEventDataTest.cpp
  lldb/unittests/Thread/CMakeLists.txt
  lldb/unittests/Thread/ThreadTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80112.269067.patch
Type: text/x-patch
Size: 49421 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200607/01f51588/attachment-0001.bin>