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

Detect whether debugger is already running and skip connecting/listen… #1657

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

oelhammouchi
Copy link
Contributor

fixes #1296

@oelhammouchi oelhammouchi requested a review from a team as a code owner August 26, 2024 18:56
@oelhammouchi
Copy link
Contributor Author

oelhammouchi commented Aug 26, 2024

@microsoft-github-policy-service agree

debugpy.connect(options.address, access_token=options.adapter_access_token)
else:
raise AssertionError(repr(options.mode))
if os.environ.get("DEBUGPY_RUNNING", "false") != "true":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why you put the flag in the environment variables instead of just a global?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I drew inspiration from the werkzeug code which distinguishes subprocesses spawned in the reloader in this way (it was werkzeug's changes that originally caused the problem). I'm not sure a global will work since it won't be set when the child process spawns, or am I wrong?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay so this is specifically the case that a subprocess shouldn't connect. I'm wondering if there's a situation that this might break. Like if you wanted to connect to subprocesses too. I believe that's what the subProcess flag is for. Would this change prevent all subProcess connections from working?

Could you instead pass --config-subprocess=false during your initial start of flask? (as described here:

-m debugpy --listen localhost:5678 --pid 12345 --configure-subProcess False
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that doesn't work, I don't think this is the fix we'd want. It seems like the reload from werkzeug should cause the original process to die before reloading. Maybe we can do that here (wait for the original server to finish if we get an address in use).

Copy link
Contributor Author

@oelhammouchi oelhammouchi Aug 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok so I'm trying to verify this use case but I can't get it working. I'm using the example app

# playground.py
from flask import Flask
from multiprocessing import Process, Queue
from time import sleep

app = Flask(__name__)


def long_calculation(queue: Queue):
    sleep(5)
    queue.put(5)


@app.route("/")
def get_data():
    queue = Queue()
    p = Process(target=long_calculation, args=(queue,))
    p.start()
    p.join()
    res = queue.get()
    return str(res), 200

which I'm running with the command

python -m debugpy --listen 127.0.0.1:5678 --configure-subProcess False -m flask -A playground run --no-debug

However, if I set a breakpoint in the helper function in VS Code and connect, it blocks at the breakpoint without showing this in the UI, so that the only way to unblock things is to shut down the program.

If I'm doing something wrong here, can you indicate how the flag is supposed to work? It's difficult to test if my fix would break this functionality without knowing how it's supposed to work in the first place 😅

Copy link
Contributor Author

@oelhammouchi oelhammouchi Aug 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it doesn't attach to child processes, I wouldn't expect it to hit a breakpoint inside a function running in such a process, but it does.

On Linux it doesn't seem to work with the latest bits, at least for me (as I commented on the issue as well).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I see what you mean about blocking the UI. If I stick a breakpoint in the subprocess code (and subProcess is false), the request never finishes. Without the breakpoint it works fine.

Seems like a separate issue, but yes it doesn't debug a child process if you pass the --configure-subProcess False. Which seems like what you'd want. I'm afraid your change would cause the same thing for all sub processes.

So with your change, can you still hit a breakpoint in the long_calculation?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried it and it seems like it works still. It doesn't force subprocess debugging off.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I can

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still works because subprocesses don't use this code path to connect. Rather, pydevd itself handles subprocesses here, such that the subprocess immediately connects to the debugpy adapter process (which then propagates this new connection to the client).

@rchiodo
Copy link
Contributor

rchiodo commented Aug 26, 2024

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rchiodo rchiodo merged commit a2f8081 into microsoft:main Aug 27, 2024
16 checks passed
@int19h
Copy link
Contributor

int19h commented Aug 28, 2024

As a general note, there is a similar problem with debugpy API if the subprocess does debugpy.listen explicitly (which happens if it's running the same exact code as the parent). It could be handled in the same way though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debugpy unable to start Flask server
3 participants