[Lldb-commits] [PATCH] D88753: Fix raciness in the check for whether a stop hook has run the target

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: teemperor, labath.
Herald added a reviewer: JDevlieghere.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
jingham requested review of this revision.

This was originally done by checking the private state to see if it was "eStateRunning" or not.  If it is running we need mark the stop event we're currently processing as "restarted" so the code like Process::ResumeSynchronous will know to keep waiting for a real stop.

But that is racy because between the time that the process gets restarted and the time we do the check, the process could have stopped again.

I used the private state because where the stop-hooks are run we are in the process of computing Public state so it won't show the effect of the restart yet.

Instead, this patch switches to just asking the stop hook whether it ran or not, and then marking the stop event as restarted or not based on the answer.  The CommandLine stop hooks know this definitively - or at least they do provided the command return status is returned correctly.  The Python stop hooks shouldn't be directly restarting the target, they should use the return values to request a restart.

I'll deal later on with the cases where (a) a command returns the wrong status and (b) somebody runs Continue/etc in a scripted stop hook even though they shouldn't by adding a mode to the Process where the high level functions that could run the target don't but instead mark an intention to continue the target.  We will turn that on when we start processing stop actions, and then when we're done with all the stop actions, check whether everybody agrees we should continue.

That will also solve the unfairness problem in the stop actions, where the first stop action that runs a "continue" command, or an SB API that continues the target causes all the other stop actions don't get a chance to run.  I've added auto-continue flags and for python return values to request running so that you CAN do the right thing.  But it would be better to make the right thing happen automatically.

But I'm leaving that for a separate patch because it's a much bigger chunk of work, and this should be sufficient for now - only failing when people do things they shouldn't do...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88753

Files:
  lldb/include/lldb/Target/Target.h
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88753.295894.patch
Type: text/x-patch
Size: 10712 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20201002/30de530b/attachment-0001.bin>