> On Sep 20, 2017, at 4:16 PM, Leonard Mosescu <mose...@google.com> wrote: > > I don't quite understand the comment about signals adding indeterminacy. No > signal delivery is required to test this part. The lldb driver has a sigint > handler that calls SBDebugger::DispatchInputInterrupt. But since you aren't > testing whether SIGINT actually calls DispatchInputInterrupt, you can just > call it directly. > > Agreed. > > I was suggesting executing a command (with > SBCommandInterpreter::ExecuteCommand) for a Python command that cycles > calling WasInterrupted. Then you would make another thread in Python and > have that thread wait a bit then call DispatchInputInterrupt, If "wait a > bit" bothers you then you could have the python based command you are > interrupting signal the interrupting thread before going into its loop. I > don't see how this would result in indeterminacy, And it would be testing > exactly what you want to test: does calling DispatchInputInterrupt cause a > command in flight to be interrupted. > > Once you have a second thread you end up with the non-determinism I hinted to > (this is true regardless if you hardcode a wait, use an event or keep > interrupting at a fixed rate). Now this is not a deal breaker in itself, > after all if you go after testing async behavior it's part of the deal. > > But in this case it gets a bit more complicated as far as I can tell: first, > DispatchInputInterrupt() is only passed on the top IO Handler, if any. So > DispatchInputInterrupt() through SBDebugger is not exactly the same as a real > input interrupt. > > Second, if we want to allow the interruption of commands that are executed > through SBDebugger::HandleCommand() the command state machine is not a simple > idle -> executing (->interrupted) -> idle since you get reentrancy (the > command can invoke other commands, etc...). Note that in the current version, > the states are only tracking in CommandInterpreter::IOHandlerInputComplete() > which should not lead to reentrancy (I did manual testing for this - if > anything I'd love a way to automate testing _this_ part btw) > > I got pretty far in dancing around all this, but it become clear that I was > not really testing the real path and I was just introducing more artificial > complexity. If I'm missing anything I'd be happy to be revisit this and to be > proven wrong.
It been a couple of years since I dug into this code - Greg’s mostly been maintaining it. So I’m entirely willing to believe it’s changed in a way that makes my memory of how it works unhelpful, but I’ll have to do some reading up to see. I’ll do that when I get a chance. Jim > > On Wed, Sep 20, 2017 at 3:51 PM, Jim Ingham via Phabricator > <revi...@reviews.llvm.org <mailto:revi...@reviews.llvm.org>> wrote: > jingham accepted this revision. > jingham added a comment. > > I don't quite understand the comment about signals adding indeterminacy. No > signal delivery is required to test this part. The lldb driver has a sigint > handler that calls SBDebugger::DispatchInputInterrupt. But since you aren't > testing whether SIGINT actually calls DispatchInputInterrupt, you can just > call it directly. I was suggesting executing a command (with > SBCommandInterpreter::ExecuteCommand) for a Python command that cycles > calling WasInterrupted. Then you would make another thread in Python and > have that thread wait a bit then call DispatchInputInterrupt, If "wait a > bit" bothers you then you could have the python based command you are > interrupting signal the interrupting thread before going into its loop. I > don't see how this would result in indeterminacy, And it would be testing > exactly what you want to test: does calling DispatchInputInterrupt cause a > command in flight to be interrupted. > > But this change is fine. Check it in and I'll add a test when I get a chance. > > > https://reviews.llvm.org/D37923 <https://reviews.llvm.org/D37923> > > > >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
