[Lldb-commits] [PATCH] D70127: [lldb-vscode] Fix a race in test_extra_launch_commands

Wed Nov 13 07:03:10 PST 2019

labath added a comment.

In D70127#1743077 <https://reviews.llvm.org/D70127#1743077>, @clayborg wrote:

> --stop-at-entry only really works for Darwin

That's not true. :) The reason this works on linux (and elsewhere) is that we don't use posix_spawn, but instead we launch the process (fork+exec) already under the control of ptrace. Ptrace automatically stops the process on execve(), so we are able to catch the process before it executes any (userspace) instructions.

In D70127#1742659 <https://reviews.llvm.org/D70127#1742659>, @jingham wrote:

> It looks like all the continue routines in lldbvscode_testcase.py use self.vscode.wait_for_stopped() after setting the target going.  Would that fix the problem here?  Without knowing anything about the protocol VSCode is using, I can't say that's right.  But if launch is asynchronous, as it looks like it is, it seems reasonable to test whether wait_for_stopped() also works for launch.  Be weird if it didn't...

I think that would fix it, but I don't know if that would test the thing that it should test (I don't know much about the LSP protocol either). I made this fix going off of the fact that lldb-vscode used eLaunchFlagStopAtEntry in a "regular" launch and that the "extra launch commands are just supposed to be a fancier form of a "regular" launch.

In D70127#1742471 <https://reviews.llvm.org/D70127#1742471>, @jankratochvil wrote:

> BTW I did not have problem with this testcase, I had problem with `TestVSCode_attach` but I never finished that patch: https://people.redhat.com/jkratoch/TestVSCode_attach.patch

Interesting. I have seen that test fail in the past, but it doesn't seem to fail now. I don't think anything substantial has changed, so likely just random perturbations made it not fail on my end. I'll keep that patch in mind if I do see it fail again.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70127