Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to retry attaching within a specific timeout #7331

Closed
bwateratmsft opened this issue Sep 11, 2019 · 27 comments
Closed

Add support to retry attaching within a specific timeout #7331

bwateratmsft opened this issue Sep 11, 2019 · 27 comments
Labels
area-debugging feature-request Request for new features or functionality

Comments

@bwateratmsft
Copy link

bwateratmsft commented Sep 11, 2019

Summary

  • Basically the debugger is launched before the preLaunch script completes. Hence the debugger fails (as the process has not started and socketserver is not listening).
  • Suggestion: Retry attaching to the socket with a default timeout period.
  • Note; This is not a bug, but a work around for the preLaunchScripts returning even before the script has actually completed (i.e. work around for docker extension issues).

Environment data

  • VS Code version: 1.38
  • Extension version: 2019.9.34911
  • OS and version: Windows 10, 1903
  • Python version: 3.7.2

Steps to reproduce:

  1. Create a Python file, e.g. myApp.py within a workspace folder
  2. Create a task in tasks.json to launch that file in PTVSD in a Docker container (see below for full tasks.json), e.g. with:
    docker run -dt -v C:\Users\brandonw\.vscode\extensions\ms-python.python-2019.9.34911\pythonFiles:/pydbg -v ${workspaceFolder}:/app -w /app -p 5678:5678 --entrypoint python python:alpine /pydbg/ptvsd_launcher.py --default --host 0.0.0.0 --port 5678 --wait myApp.py
  3. Set that task as the preLaunchTask for a Python remote attach launch config (see below for full launch.json)
  4. F5

Expected behaviour

Attach works if preLaunchTask is used to start a waiting debug server with ptvsd

Actual behaviour

Attach does not work. No error is shown, it appears like debugging starts and immediately stops. If you comment out the preLaunchTask in launch.json, and then manually run the task with Ctrl + Shift + P > Run Task, it will start the container. If you then F5 it attaches successfully. I've also tried putting a dummy task containing a 2-second sleep inbetween, and this also works. So it seems to just be a race condition.

Logs

Nothing is output in Python output window, nor in Developer Tools console window.

Workspace

.vscode/launch.json:

{
    // Use IntelliSense to learn about possible attributes.
    // Hover to view descriptions of existing attributes.
    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
    "version": "0.2.0",
    "configurations": [
        {
            "name": "Python: Remote Attach",
            "type": "python",
            "request": "attach",
            "port": 5678,
            "host": "localhost",
            "pathMappings": [
                {
                    "localRoot": "${workspaceFolder}",
                    "remoteRoot": "."
                }
            ],
            "preLaunchTask": "Docker start",
            "postDebugTask": "Docker remove"
        }
    ]
}

.vscode/tasks.json (note, you'll need to change the C:\users\brandonw...\pythonFiles:/pydbg path):

{
    "version": "2.0.0",
    "tasks": [
        {
            "label": "Docker start",
            "type": "shell",
            "command": "docker run -dt -v C:\\Users\\brandonw\\.vscode\\extensions\\ms-python.python-2019.9.34911\\pythonFiles:/pydbg -v ${workspaceFolder}:/app -w /app -p 5678:5678 --name myPythonApp --entrypoint python python:alpine /pydbg/ptvsd_launcher.py --default --host 0.0.0.0 --port 5678 --wait myApp.py",
            "problemMatcher": []
        },
        {
            "label": "Docker remove",
            "type": "shell",
            "command": "docker rm -f myPythonApp"
        }
    ]
}

myApp.py:

print("Hello world!")

print("Farewell!")
@bwateratmsft bwateratmsft added triage-needed Needs assignment to the proper sub-team bug Issue identified by VS Code Team member as probable bug labels Sep 11, 2019
@bwateratmsft
Copy link
Author

Blocks microsoft/vscode-docker#1255

@DonJayamanne
Copy link

DonJayamanne commented Sep 11, 2019

Isn't the problem the fact that the preLaunch task returns even before the script has completed. I.e. it's not really an issue with the python extension. However an issue with the preLaunch script.

preLaunch functionality isn't something that's provided by the extension. The extension is doing what it's meant to do. It starts the debugger when VSCode yes it when it's time to do so.

I.e. the race condition is VSCode specific or due to the preLaunch retuning too early

@bwateratmsft
Copy link
Author

bwateratmsft commented Sep 11, 2019

@DonJayamanne VSCode does actually wait until docker run has finished, but I don't think that docker run waits for python to get up, running, and ready in the container before returning.

@bwateratmsft
Copy link
Author

Since it's expected that the entrypoint is a long-running application, the -d option (detached) would mean it doesn't wait for the application to return. We also can't do -i (interactive) as then the task would never return.

@DonJayamanne
Copy link

DonJayamanne commented Sep 11, 2019

but I don't think that docker run waits for python to get up, running, and ready in the container before returning.

Not sure how this is a bug in the extension. We aren't in control of the preLaunch task or any of that orchestration!

@DonJayamanne
Copy link

Same problem could happen with other debuggers.
Either you'd need to find a way to synchronize the two, or find an alternative. (E.g use custom tasks in vscode), or a custom debugger when you manually start docker and that start the debugger using the VSCode API).

@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Sep 11, 2019
@DonJayamanne DonJayamanne reopened this Sep 11, 2019
@ghost ghost added the triage-needed Needs assignment to the proper sub-team label Sep 11, 2019
@bwateratmsft
Copy link
Author

It's not a bug, but it's a shortcoming that can't be addressed by Docker itself or by the Docker VSCode extension (they have no idea what constitutes 'the debugger listener is ready' or how to detect it).

For comparison, the Node.js debugger has a timeout parameter that can help in this sort of scenario: https://github.com/microsoft/vscode-node-debug2/blob/965054f347a8f4ea4a690f82083e9ad8f708633e/package.json#L301

@DonJayamanne DonJayamanne added feature-request Request for new features or functionality and removed bug Issue identified by VS Code Team member as probable bug labels Sep 11, 2019
@DonJayamanne
Copy link

DonJayamanne commented Sep 11, 2019

Updated label as enhancement (confused as you originally filled it as a bug).
Again it's basically a problem with the oreLaunch talk and it's shortcomings.

@bwateratmsft
Copy link
Author

Here's the problem--this blocks all remote attach scenarios if you want to start up the remote process with a preLaunchTask. It is not limited to just Docker.

There are only two choices--either you wait on python ptvsd_launcher.py ..., in which case you'll wait forever and preLaunchTask cannot be used; or you don't wait, in which case you can't be certain that it'll be ready by the time the Python debugger tries to attach.

For VSCode-Docker in particular, if we had VSCode's CustomExecution we could introduce a sleep after launching the container, but that API is still proposed and has been since March. Without that API there's no way to do a sleep other than to explicitly put a dummy sleep task as the preLaunchTask, which is a pretty ugly user experience. Nevertheless, sleeping is a clunky workaround to the problem.

@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Sep 11, 2019
@DonJayamanne DonJayamanne changed the title Attach can happen too quickly after preLaunchTask and doesn't retry Add support to retry attaching within a specific timeout Sep 13, 2019
@DonJayamanne
Copy link

Proposed solution

  • *Retry attaching to the socket with a default timeout period.
    Note; This is not a bug, but a work around for the preLaunchScripts returning even before the script has actually completed (i.e. work around for docker extension issues).

@DonJayamanne
Copy link

DonJayamanne commented Sep 13, 2019

@bwateratmsft
Have you looked at using the HEALTHCHECK option to wait for the port to be opened.

If you incorporate this, then there's no need to modify the extension. Makes it more bullet proof (no need to change, node, python and other language specific extension to get around this issue with preLaunchScript in the docker extension).

E.g. creating a custom debugger in docker extension would also resolve this. This way docker extension can wait for the port to be opened.
I'm looking for generic fix that wouldn't require all other extensions to add a timeout/retry option (it is too specific to docker).

@bwateratmsft
Copy link
Author

bwateratmsft commented Sep 13, 2019

I have looked into health checks, but there's still problems:

  1. docker run doesn't wait for health checks before returning, so we would still need a task inbetween, which is a terrible UX.
  2. Knowing if the debugger is ready shouldn't be something that we, and every other user facing this problem, have to do. The port being open is not enough. Docker starts the container with the port open. I have no idea how to determine if the debugger is ready.
  3. This doesn't solve the problem for anything other than specifically our Docker extension's scenario. Anyone else wanting to start a remote with a preLaunchTask has to reimplement the wheel.

Again, this is not specific to Docker. This is any scenario where the user wants to do remote debugging and use a preLaunchTask to start the remote debugger.

Why would any other extension need to make changes? If the Python debug extension had a timeout option, just like the Node debug extension does, then we could leverage that timeout in our resolved configuration.

@DonJayamanne
Copy link

DonJayamanne commented Sep 14, 2019

Why would any other extension need to make changes? If the Python debug extension had a timeout option, just like the Node debug

Java, C#, and other languages would have similar issues when waiting for ports to open...

I'm just looking for a way to resolve this in a generic way, without having to add retires, that sounds hacky.

Anyways based in what you're saying it doesn't sound like we have a choice.

@bwateratmsft
Copy link
Author

I haven't looked into Java yet so I can't speak for it, but for .NET debugging retry is already built in between vsdbg and the debug adapter. In any case our usage is primarily Node, with .NET Core and Python in second and third. No other languages come even close to these three.

@bwateratmsft
Copy link
Author

bwateratmsft commented Sep 17, 2019

I got one part about .NET Core wrong; we don't launch vsdbg inside then attach, rather we launch it interactively with docker exec -i, then we (well, OmniSharp) instruct it on how to launch the actual .NET Core app. It'd be nice if we could do something like that for Python but I understand that would be a much, much larger change. Attach + ptvsd with --wait is essentially the same thing from a user-experience perspective.

@DonJayamanne
Copy link

DonJayamanne commented Sep 17, 2019

@karthiknadig @int19h /cc
Cc'ing, in case you have any suggestions, apart from the work around of adding a timeout/retry.

@int19h
Copy link

int19h commented Sep 17, 2019

I wonder if it would help if you could reverse the direction of the connection? i.e. from Docker to VSCode, rather than the other way around? Basically the order for the debugger bits would be:

  1. User (or extension) starts debug session.
  2. IDE waits for incoming connection from debuggee.
  3. Debuggee starts, connects to IDE at the specified port.
  4. Debugging proceeds.

It sounds like this would solve the prelaunch problem?

@bwateratmsft
Copy link
Author

bwateratmsft commented Sep 17, 2019

@int19h I'm not really sure how that works, can you elaborate?

We are constrained by the order of execution that VSCode does things with (debug config resolved, preLaunchTask(s) executed, debug config 'executed'). In our case, it's this:

  1. Debug config resolved, i.e. DebugConfigurationProvider.resolveDebugConfiguration
  2. docker build task
  3. docker run task
  4. Debug config 'executed'; I don't know a whole lot about debug adapters though so I'm not sure what all happens here

Would it still be possible to do what you're describing, while constrained with that order? I'd definitely be interested to hear more about this approach.

EDIT: rephrasing for clarity

@bwateratmsft
Copy link
Author

bwateratmsft commented Sep 17, 2019

In particular, one thing I do want to avoid is cramming everything into DebugConfigurationProvider.resolveDebugConfiguration. We recently spent a lot of time refactoring our whole debugging flow so that docker build and docker run could become tasks (previously, we had shoehorned them into DebugConfigurationProvider.resolveDebugConfiguration which is a bit of an abuse of that API--though at the time it was the only option, since resolveTask wasn't actually implemented in VSCode yet).

@int19h
Copy link

int19h commented Sep 17, 2019

To clarify the specifics. We have an (experimental, not officially advertised or supported yet) feature in our prerelease bits where you can do this in debug config in launch.json:

"request": "attach",
"host": ...,
"port": ...,
"listen": true,

If you do this and then start the debug session, it will sit there and wait for the incoming connection. And you can tell ptvsd to initiate by either using --client in CLI, or using ptvsd.attach() instead of ptvsd.enable_attach() in API.

On the protocol level, what happens there is that VSCode will send the "attach" request, but it won't receive "initialized" event or the "attach" response until there's a socket connection from the adapter to the debuggee.

So if I'm understanding everything correctly, the way this will work in your scenario is that the preLaunchScript will return before the actual script completes, and move on to start the debug session. But the debug session, once started, will then block waiting for connection, and that will only happen once the script completes and starts the debuggee. So you wouldn't really need to sync anything explicitly.

@bwateratmsft
Copy link
Author

It is definitely possible that the the client part in the container starts up before the listener is ready on the host--would that fail as it does currently? If so we'd still have the race condition, just swapping the client/server.

@int19h
Copy link

int19h commented Sep 17, 2019

Ahh, I see what you mean now. Yes, it would also be a race condition...

Can you clarify how this flow works with vsdbg.exe? Specifically, at which step do you launch it, such that it is guaranteed to be there and listening by the time the debug session starts? Our new adapter design has an out-of-process bit as well, and I'm thinking that it might be possible to rig it to be used in a similar way, but it depends on the details.

@bwateratmsft
Copy link
Author

For .NET Core / vsdbg, it looks like this:

  1. Resolve config (we resolve to coreclr)
  2. The C# extension now gets a hold of it, and does whatever they do when resolving
  3. docker build
  4. docker run, but this starts a container that's doing nothing--just alive and waiting
  5. Executing the config--the C# extension has a bunch of pipeTransport options, which are basically a thing you execute that gets you to vsdbg. For us, pipeProgram=docker and pipeArgs=exec -i <containerName> /remote_debugger/vsdbg. This docker exec command is an interactive process, so stdio, stdout, and stderr are piped up. Once this is running, the C# extension / debug adapter tells vsdbg (through stdio) to start the actual .NET Core app itself, via dotnet /path/to/app.dll.

@int19h
Copy link

int19h commented Sep 18, 2019

So, basically the C# extension lets you customize the command line for spawning the adapter, which allows it to be spawned remotely inside the container via the appropriate helper - and then it's really a "launch" scenario, where the remote adapter starts the process within the container?

It sounds like this approach could be reused. We spawn our new out-of-proc adapter via DebugAdapterDescriptor and DebugAdapterExecutable. That code could also allow similar customization, maybe even using the same property names. And it sounds like that's the only bit that would need to change? docker exec would take care of moving the bits back and forth, and as far as VSCode is concerned, it's still dealing with a local process and its stdin/out. The actual adapter is similarly oblivious.

@bwateratmsft
Copy link
Author

bwateratmsft commented Sep 18, 2019

Yes, for C# it truly is a 'launch' scenario instead of 'attach'. That approach would work great for us, if you're already working toward it anyway. If it helps, here's where their settings start: https://github.com/OmniSharp/omnisharp-vscode/blob/6405d17613152249d820a11e1120d03fdc8f20df/package.json#L1123

And here's an example of what we resolve (some irrelevant bits removed for brevity) that results in launching 'TestWeb.dll':

{
    "name": "Docker .NET Core Launch",
    "preLaunchTask": "docker-run: debug",

    "pipeTransport": {
        "debuggerPath": "/remote_debugger/vsdbg",
        "pipeProgram": "docker",
        "pipeArgs": [
            "exec",
            "-i",
            "testsln-dev",
            "${debuggerCommand}"
        ],
        "pipeCwd": "${workspaceFolder}",
        "quoteArgs": false
    },

    "program": "dotnet",
    "args": "--additionalProbingPath /root/.nuget/packages --additionalProbingPath /root/.nuget/fallbackpackages /app/bin/Debug/netcoreapp3.0/TestWeb.dll",
    "cwd": "/app",

    "request": "launch",
    "type": "coreclr"
}

Of note, the C# extension will call pipeProgram + pipeArgs from within pipeCwd, (${debuggerCommand} is essentially just ${debuggerPath} --interpreter=vscode). So it does this:
${pipeProgram} ${pipeArgs} ${debuggerCommand}, resulting in
docker exec -i testsln-dev /remote_debugger/vsdbg --interpreter=vscode

Then, to VSDBG, it instructs it on launching the process, with program + args from within cwd:
${program} ${args}, resulting in
dotnet --additionalProbingPath /root/.nuget/packages --additionalProbingPath /root/.nuget/fallbackpackages /app/bin/Debug/netcoreapp3.0/TestWeb.dll.

Our actual code for this starts here: https://github.com/microsoft/vscode-docker/blob/35513c85ade31e0b6c6e8924f3055dbe828dd174/src/debugging/netcore/NetCoreDebugHelper.ts#L140

As far as vsdbg is concerned, it's just launching a dotnet ...TestWeb.dll process, and happens to be running inside a Linux container; and as far as VSCode is concerned, it's debugging with vsdbg the normal way--through stdio/stdout/stderr.

@luabud
Copy link
Member

luabud commented Oct 27, 2020

Closing as stale.

@luabud luabud closed this as completed Oct 27, 2020
@ghost ghost removed the needs PR label Oct 27, 2020
@vintlucky777
Copy link

Hey guys! I have a different usecase, but could benefit from debugger attach retry.

I've got a Django app running in a docker-dompose cluster, launched with watchmedo to enable quick code reload on file changes. Watchmedo restarts the Django process completely on code change, but that also means the debugger session disconnects.

How an I configure debugger launch configuration, so that it would try to reconnect debugger session, when Django process restarts? It takes about 5-10 secs to restart Django process and relaunch ptvsd.

In my current setup, Django spins up with docker-compose up on the side, exposes ports 8000 and 8001.
ptvsd is enabled in local debug mode:

# manage.py

if __name__ == "__main__":
    from django.conf import settings
    from django.core.management import execute_from_command_line

    if settings.DEBUG:
        if os.environ.get('DJANGO_SETTINGS_MODULE').split('.')[-1] == 'docker-local':
            DEBUG_PORT = 8001
            import ptvsd
            ptvsd.enable_attach(address=('0.0.0.0', DEBUG_PORT))
            print(f'VSCode debug tools attached. Listening on port {DEBUG_PORT}')

    execute_from_command_line(sys.argv)

Debugger launch configuration:

{
  "version": "0.2.0",
  "configurations": [
    {
      "name": "Django: Docker debug",
      "type": "python",
      "request": "attach",
      "pathMappings": [
        {
          "localRoot": "${workspaceFolder}",
          "remoteRoot": "/src"
        }
      ],
      "port": 8001,
      "host": "127.0.0.1",
      "listen": {
        "port": 8001,
        "host": "127.0.0.1",
      }
    }
  ]
}

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-debugging feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

6 participants