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

customRequest 'processId' to get the debugging Java process id #399

Merged
merged 5 commits into from
Mar 28, 2022

Conversation

testforstephen
Copy link
Contributor

@testforstephen testforstephen commented Feb 21, 2022

request: 'processId'
response:
{
processId?: long; // The process id
shellProcessId?: long; // The process id of the terminal if the Java process is running in a terminal
}

Also the server sends a custom notification when processId/shellProcessId is ready.

@@ -125,6 +126,7 @@ private void initialize() {
registerHandlerForDebug(new SetDataBreakpointsRequestHandler());
registerHandlerForDebug(new InlineValuesRequestHandler());
registerHandlerForDebug(new RefreshVariablesHandler());
registerHandlerForDebug(new ProcessIdHandler());

// NO_DEBUG mode only
registerHandlerForNoDebug(new DisconnectRequestWithoutDebuggingHandler());

Choose a reason for hiding this comment

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

When I debug Boot App via "Debug" action i get this back:

{"processId":0,"shellProcessId":78134}

which is great.

However, when i run the app via the "Run" action i get an error back:

Error: {"name":"Error","message":"custom request failed"}

Is it possible to make it work with "Run" action?

I thought the change for this would be instead of registerHandlerForDebug(...) call registerHandler(...) but this didn't work for me and i hope also because i messed up the build...

What do you think?

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, we can support it in "Run" as well.

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 have added 'processId' request in Run action as well.

If the Run action is running with internalConsole, it works fine.

However, if the Run action is triggered on terminal shell, it may not work. In this case, Java debugger will terminate the DAP debug session early once the RunInTerminal request is responded. You may not have opportunity to call custom request to get processId.

We have a TODO task to not terminate DAP session when triggering a Run with integratedTerminal, which requires more work and may not be done right away.

Eskibear
Eskibear previously approved these changes Feb 24, 2022
Copy link
Member

@Eskibear Eskibear left a comment

Choose a reason for hiding this comment

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

LGTM.

@BoykoAlex
Copy link

@testforstephen Sorry for getting back to you only now... Was away from work and then had other projects to take care of.
I've tried the support for run action (hope i did everything correctly building it).
Here is snippet of code printing response for processId custom request:

    VSCode.debug.onDidStartDebugSession((e: DebugSession) => {
        if (e.type === 'java') {
            console.log('Started debug session for ' + e.configuration.mainClass);
            e.customRequest('processId').then(r => console.log(`resp ${JSON.stringify(r)}`));
        }
    });

Now, when i debug the app i get this printed:

Started debug session for org.springframework.samples.petclinic.PetClinicApplication
resp {"processId":0,"shellProcessId":85334}

However, when i run the app I get this:

Started debug session for org.springframework.samples.petclinic.PetClinicApplication
rejected promise not handled within 1 second: Error: custom request failed
/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:104
stack trace: Error: custom request failed
    at [vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:1520:7618]()
    at processTicksAndRejections (internal[/process/task_queues.js:93:5]())

Looks to me that this request is never even sent to DAP... Any idea what is going on here?

@testforstephen
Copy link
Contributor Author

if the Run action is triggered on terminal shell, it may not work. In this case, Java debugger will terminate the DAP debug session early once the RunInTerminal request is responded. You may not have opportunity to call custom request to get processId.

Like I mentioned before, for Run action, the debug session only exists for a very short moment. Once the terminal is started, Java debugger will send a terminate event back to VS Code to exit debug session. That's why custom request is rejected for your case.

@testforstephen
Copy link
Contributor Author

Another idea is to make the debugger to send a custom event with the processId/shellProcessId as payload. This approach works both for Run/Debug.

And the client can use the code like vscode.debug.onDidReceiveDebugSessionCustomEvent((customEvent) => <callback>) to listen for the custom event.

@BoykoAlex
Copy link

@testforstephen Just tried custom notifications. Think this would be ideal solution for me :-)

@testforstephen
Copy link
Contributor Author

@BoykoAlex The latest commit will send a custom notification when processId/shellProcessId is ready.

@BoykoAlex
Copy link

I have tried the latest version of this PR that includes the custom processid events. Generally it worked well. Generally i see only shellProcessId present and I attempt to find a child process with ppid === shellProcessId. Sometimes, however, there is no child process found with parent process id being shellProcessId. I worked around it with setTimeout(..., 500). If there is a way to send the processid event after the java app process guaranteed to be created that'd be great. Otherwise, I think, we can live with my workaround perhaps...

@testforstephen
Copy link
Contributor Author

I have tried the latest version of this PR that includes the custom processid events. Generally it worked well. Generally i see only shellProcessId present and I attempt to find a child process with ppid === shellProcessId. Sometimes, however, there is no child process found with parent process id being shellProcessId. I worked around it with setTimeout(..., 500). If there is a way to send the processid event after the java app process guaranteed to be created that'd be great. Otherwise, I think, we can live with my workaround perhaps...

I don't know how do you test this, it's supposed to report the process info after the debug connection is setup.

@testforstephen
Copy link
Contributor Author

We're going to have a release this week. I plan to merge this PR first. If there is any further improvement needed, we can send another PR.

@testforstephen testforstephen merged commit 8a1c975 into microsoft:main Mar 28, 2022
@testforstephen testforstephen deleted the jinbo_processId branch March 28, 2022 04:48
@BoykoAlex
Copy link

@testforstephen Here is how i test it:

// call in extension activate function
VSCode.debug.onDidReceiveDebugSessionCustomEvent(handleCustomDebugEvent);

async function handleCustomDebugEvent(e: VSCode.DebugSessionCustomEvent): Promise<void> {
    if (e.session?.type === 'java' && e?.body?.type === 'processid') {
        console.log('Event: ' + JSON.stringify(e.body));
        const debugConfiguration: DebugConfiguration = e.session.configuration;
        // Sometimes spring boot app (child process) is not yet created by the time the event is received
        // Therefore give it 500ms - should be more than enough to start the child process 
        setTimeout(async () => {
            const pid = await getAppPid(e.body as ProcessEvent);
            const processKey = pid.toString();
            console.log('Command: ' + processKey);
        }, 500);
    }
}

async function getAppPid(e: ProcessEvent): Promise<number> {
    if (e.pid) {
        return e.pid;
    } else if (e.shellProcessId) {
        const processes = await psList();
        const appProcess = processes.find(p => p.ppid === e.shellProcessId);
        if (appProcess) {
            return appProcess.pid;
        }
        throw Error(`No child process found for parent shell process with pid = ${e.shellProcessId}`);
    } else {
        throw Error('No pid or parent shell process id available');
    }
}

psList() comes from: import psList from 'ps-list';

@Eskibear
Copy link
Member

I can also repro the issue mentioned in #399 (comment) , with another lib pidtree to fetch subprocesses of the terminal.

image

But I think java-debug has no knowledge about when the real java process will be alive.

@testforstephen
Copy link
Contributor Author

Recently I'm also working on improving the processId mechanism in debugger side, so that it returns the correct Java process pid directly instead of the shellProcessId.

On Windows, I found in some terminal environment, the started Java process is not a child process of the returned shell process.

  • Using PowerShell as the default terminal, the started process is a child of the shell process.
  • Using cmd as the default terminal, VS Code will use the syntax cmd C/ "java.exe ..." to start Java process, that will make Java process is started under a new cmd.exe process. So the Java process is a grandchild of the shell process.
  • Using Git Bash as the default terminal, VS Code will use the syntax /usr/bin/env java.exe ... to start Java process, that will make Java process is a child of env.exe. The weird part is that env.exe is not a descendant of Git Bash process. We have to explore new way to get Java pid for the case of Git Bash.

On macOS, the started process is always a child of the target shell terminal.

@testforstephen
Copy link
Contributor Author

Using Git Bash as the default terminal, VS Code will use the syntax /usr/bin/env java.exe ... to start Java process, that will make Java process is a child of env.exe. The weird part is that env.exe is not a descendant of Git Bash process. We have to explore new way to get Java pid for the case of Git Bash.

After some investigation, I found this is related with Cygwin emulator, which is a technology to run Linux GNU apps in Windows. When running Cygwin exe in Git Bash, it runs at an emulator layer and its process tree is not same as Windows process tree. However, each Cygwin process has a mapped Windows pid. Running ps command in Git Bash can list the cygwin's pid, cygwin's ppid and its winpid. Based on ps command, it's possible to find the running Java process and its parent Git bash process.

The new PR #413 includes the improvement to return the Java processId directly when running Java in terminal.
// cc: @BoykoAlex

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.

3 participants