aadsm marked an inline comment as done.
aadsm added inline comments.
================
Comment at:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py:250-251
def cleanup():
- self.vscode.request_disconnect(terminateDebuggee=True)
+ if disconnect:
+ self.vscode.request_disconnect(terminateDebuggee=True)
self.vscode.terminate()
----------------
aadsm wrote:
> labath wrote:
> > aadsm wrote:
> > > labath wrote:
> > > > What's the purpose of this argument? To ensure a clean shutdown? Would
> > > > it be possible to make the function smart enough to detect the right
> > > > thing to do when cleaning up?
> > > it indicates if the process should be killed or not when lldb-vscode
> > > disconnects from it.
> > I meant the `disconnect` argument to the `attach` function in the test
> > suite , not the `terminateDebuggee` argument of the `disconnect` command to
> > lldb-vscode.
> >
> > My point is that, given the way the cleanup functions work, you cannot
> > assume that you're going to be invoked in any particular moment. For
> > example they can called if an assertion fails, to ensure any external
> > resources/processes are terminated. Since you don't know which assertion
> > will fail, it's best if you don't assume anything about the the state of
> > the inferior and just do your best to clean up. I'm not sure what "doing
> > your best" would mean here. For example it may mean sending a "terminate"
> > request and ignoring errors. Or maybe a better solution is possible...
> oh yeah of course, the argument I added 馃槄. Oh, you know what, I misread when
> the cleanup function was going to be called. I don't need this at all, will
> remove it.
sorry about this, I remember now (I've been working on something else not
related to this when I replied).
I did it this way so I can explicitly say that I don't want the test to
automatically disconnect when it's teared down. I need to disconnect manually
so I can test the terminateCommands, and and 2 disconnects won't work well
because we'll be waiting for that answer that will never arrive, since it has
already been terminated.
Another way to do this, that I thought at the time, was to track if the test is
connected or not and only disconnect in that situation. I opted to not do that
in the end because I wanted this behaviour to be intentional in the test, to
prevent something from disconnecting (unintentionally, like a bug) and then the
teardown will skip it and that error would never be surfaced. This might be a
bit overzealous though...
what do you think?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79726/new/
https://reviews.llvm.org/D79726
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
