jingham added a comment.

The part of handling the fork where we decide we're going to follow the child 
and so we need to switch the process PID & TID does have to happen on event 
receipt.  The point there is that until the client pulls an event from the 
public event queue, it doesn't know that the state change has happened yet.  We 
don't want the current state that you access with GetState, GetPID, 
GetSelectedThread, etc. to get out of sync with the event flow, so to the 
public world it should appear to it as if the process were in the state of the 
last event it actually received.  Having the PID or TID change before the fork 
event gets pulled off the queue would break this illusion.

However, the state of the process we aren't following isn't relevant to the 
state of the process we are  following, so the part of the DidFork dealing with 
the discarded side of the fork can happen when the fork event is received, in 
ProcessGDBRemote::SetThreadStopInfo or thereabouts.  That does seem like a 
better way to do this.

However, I don't think this patch is wrong in itself.  Formally, the step 
thread plans might want to know that a fork has happened.  But it's not 
terribly important as they can't really do anything about except maybe report 
an error.  And the same could be said of exec events which are already in this 
list.  What's actually going to happen is once we've stopped for the fork, all 
the step plans will be asked whether they are Stale, and given the thread they 
were on is no longer present, they should unship themselves.  So we're not 
losing anything by bypassing the plans for this event delivery.

Note, it might be worth checking that the stepping thread plan's ShouldStop 
check the  PID, not just the TID.  It's possible that the new TID was the same 
as the old TID and the plan might try to keep operating, which would be wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141605

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to