<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Sep 20, 2017, at 4:16 PM, Leonard Mosescu <<a href="mailto:mosescu@google.com" class="">mosescu@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px" class="">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::</span><wbr style="font-size:12.8px" class=""><span style="font-size:12.8px" class="">DispatchInputInterrupt. But since you aren't testing whether SIGINT actually calls DispatchInputInterrupt, you can just call it directly. </span></blockquote><div class=""><span style="font-size:12.8px" class=""><br class=""></span></div><div class=""><span style="font-size:12.8px" class="">Agreed.</span></div><div class=""><span style="font-size:12.8px" class=""><br class=""></span></div><div class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px" class="">I was suggesting executing a command (with SBCommandInterpreter::</span><wbr style="font-size:12.8px" class=""><span style="font-size:12.8px" class="">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.</span></blockquote><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">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)</div><div class=""><br class=""></div><div class="">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.</div></div></div></div></blockquote><div><br class=""></div><div>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.</div><div><br class=""></div><div>Jim</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Wed, Sep 20, 2017 at 3:51 PM, Jim Ingham via Phabricator <span dir="ltr" class=""><<a href="mailto:reviews@reviews.llvm.org" target="_blank" class="">reviews@reviews.llvm.org</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">jingham accepted this revision.<br class="">
jingham added a comment.<br class="">
<br class="">
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::<wbr class="">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::<wbr class="">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.<br class="">
<br class="">
But this change is fine. Check it in and I'll add a test when I get a chance.<br class="">
<br class="">
<br class="">
<a href="https://reviews.llvm.org/D37923" rel="noreferrer" target="_blank" class="">https://reviews.llvm.org/<wbr class="">D37923</a><br class="">
<br class="">
<br class="">
<br class="">
</blockquote></div><br class=""></div></div></div>
</div></blockquote></div><br class=""></body></html>