Report more detail in VM start errors #398

karlvr · 2022-01-25T23:23:29Z

Resolves #397 or at least contributes to it

ghost · 2022-01-25T23:23:41Z

All CLA requirements met.

testforstephen · 2022-01-26T03:26:08Z

This is a nice improvement.

Currently the CI reports some checkstyle errors, you can run checkstyle check via cmd ./mvnw checkstyle:check in local machine.

Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[1] (header) RegexpHeader: Line does not match expected header line of '^/\*+$'.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[10] (regexp) RegexpSingleline: Line has trailing spaces.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[10,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[11] (indentation) Indentation: 'member def type' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[11,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[12] (indentation) Indentation: 'member def type' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[12,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[13] (indentation) Indentation: 'member def type' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[13,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[14] (indentation) Indentation: 'member def type' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[14,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[16] (indentation) Indentation: 'ctor def modifier' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[16,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[17] (indentation) Indentation: 'ctor def' child has incorrect indentation level 16, expected level should be 8.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[17,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[18] (indentation) Indentation: 'ctor def' child has incorrect indentation level 16, expected level should be 8.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[18,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[19] (indentation) Indentation: 'ctor def' child has incorrect indentation level 16, expected level should be 8.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[19,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[20] (indentation) Indentation: 'ctor def' child has incorrect indentation level 16, expected level should be 8.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[20,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[21] (indentation) Indentation: 'ctor def' child has incorrect indentation level 16, expected level should be 8.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[21,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[22] (indentation) Indentation: 'ctor def rcurly' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[22,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[24] (indentation) Indentation: 'method def modifier' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[24,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[25] (indentation) Indentation: 'method def' child has incorrect indentation level 16, expected level should be 8.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[25,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[26] (indentation) Indentation: 'method def rcurly' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[26,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[28] (indentation) Indentation: 'method def modifier' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[28,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[29] (indentation) Indentation: 'method def' child has incorrect indentation level 16, expected level should be 8.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[29,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[30] (indentation) Indentation: 'method def rcurly' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[30,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[32] (indentation) Indentation: 'method def modifier' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[32,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[33] (indentation) Indentation: 'method def' child has incorrect indentation level 16, expected level should be 8.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[33,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[34] (indentation) Indentation: 'method def rcurly' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[34,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[36] (indentation) Indentation: 'method def modifier' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[36,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[37] (indentation) Indentation: 'method def' child has incorrect indentation level 16, expected level should be 8.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[37,1] (whitespace) FileTabCharacter: Line contains a tab character.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[38] (indentation) Indentation: 'method def rcurly' has incorrect indentation level 8, expected level should be 4.
Error:  src/main/java/com/microsoft/java/debug/core/LaunchException.java:[38,1] (whitespace) FileTabCharacter: Line contains a tab character.

karlvr · 2022-01-26T06:55:42Z

@testforstephen thank you. Just a few checkstyle errors there! I've force-pushed a fix for them to keep the commit history clean.

I have been investigating terminating early if the process exits... I don't know if there are any clever tricks to do that here, I haven't done any work with Futures! I'm sort of looking for a "race" between the Process exiting and the debug connection accept returning... also given that accept will timeout. Any suggestions welcome otherwise I'll continue to experiment.

testforstephen · 2022-01-26T07:38:23Z

java-debug/com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/AdvancedLaunchingConnector.java

Lines 86 to 95 in e6655ea

    
           Process process = Runtime.getRuntime().exec(cmds, envVars, workingDir); 
        
           VirtualMachineImpl vm; 
        
           try { 
        
               vm = (VirtualMachineImpl) listenConnector.accept(args); 
        
           } catch (IOException | IllegalConnectorArgumentsException e) { 
        
               process.destroy(); 
        
               throw new VMStartException(String.format("VM did not connect within given time: %d ms", ACCEPT_TIMEOUT), process); 
        
           }

How about using process.onExit() to watch the exit event and return early?

        process.onExit().whenCompleteAsync((p, ex) -> {
            if (vm == null) {  // When the debug connection is not set up yet.
                  listenConnector.stopListening(args);
                  ...
            }
        });

karlvr · 2022-02-05T00:23:35Z

@testforstephen I have pushed some additional work that detects early termination of the launched process, so we no longer have to wait for the timeout to report the error.

testforstephen · 2022-02-14T11:34:23Z

Just came back from vacation, I will find some time this week to review this PR?

BTW, do you have any sample project to help reproduce the issue at #397? thanks.

testforstephen · 2022-02-18T04:50:10Z

...lugin/src/main/java/com/microsoft/java/debug/plugin/internal/AdvancedLaunchingConnector.java

+        final CompletableFuture<VirtualMachineImpl> result = new CompletableFuture<>();
+
+        /* Listen for the debug connection from the Java process */
+        ForkJoinPool.commonPool().execute(() -> {


Prefer to use CompletableFuture.runAsync(...)

testforstephen · 2022-02-18T05:00:31Z

...lugin/src/main/java/com/microsoft/java/debug/plugin/internal/AdvancedLaunchingConnector.java

+        });
+
+        /* Wait for the Java process to exit; if it exits before the debug connection is made, report it as an error. */
+        process.onExit().thenAcceptAsync(theProcess -> {


In real world, is there any use case that the process has exited but the listenConnector is still keeping the connection and not terminate?

@testforstephen I'm not sure. I have seen socket-related things not detecting disconnection etc. I tried to tidy everything up in each case. Is there a particular case that you're thinking may be overkill?

I see process.onExit() is hit immediately with your sample. Looks good.

karlvr · 2022-03-12T03:08:19Z

@testforstephen I hope you had a great vacation. I have added a very simple example of reproducing the error to #397.

testforstephen

LGTM. @karlvr thanks for the contribution.

testforstephen self-requested a review January 26, 2022 02:52

Report more detail in VM start errors

2535217

karlvr force-pushed the launch-error-reporting branch from 009ead8 to 2535217 Compare January 26, 2022 06:24

Detect VM start failures before the timeout waiting for debug connection

4ca695a

karlvr force-pushed the launch-error-reporting branch from fe233f1 to 4ca695a Compare February 4, 2022 23:55

testforstephen reviewed Feb 18, 2022

View reviewed changes

Prefer to use CompletableFuture.runAsync

4916dbd

testforstephen approved these changes Mar 17, 2022

View reviewed changes

Merge branch 'main' into launch-error-reporting

593e3ee

testforstephen merged commit 7f6f76b into microsoft:main Mar 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Report more detail in VM start errors #398

Report more detail in VM start errors #398

karlvr commented Jan 25, 2022

ghost commented Jan 25, 2022 •

edited by ghost

Loading

testforstephen commented Jan 26, 2022

karlvr commented Jan 26, 2022

testforstephen commented Jan 26, 2022

karlvr commented Feb 5, 2022

testforstephen commented Feb 14, 2022

testforstephen Feb 18, 2022

testforstephen Feb 18, 2022

karlvr Mar 12, 2022

testforstephen Mar 17, 2022

karlvr commented Mar 12, 2022

testforstephen left a comment

Report more detail in VM start errors #398

Report more detail in VM start errors #398

Conversation

karlvr commented Jan 25, 2022

ghost commented Jan 25, 2022 • edited by ghost Loading

testforstephen commented Jan 26, 2022

karlvr commented Jan 26, 2022

testforstephen commented Jan 26, 2022

karlvr commented Feb 5, 2022

testforstephen commented Feb 14, 2022

testforstephen Feb 18, 2022

Choose a reason for hiding this comment

testforstephen Feb 18, 2022

Choose a reason for hiding this comment

karlvr Mar 12, 2022

Choose a reason for hiding this comment

testforstephen Mar 17, 2022

Choose a reason for hiding this comment

karlvr commented Mar 12, 2022

testforstephen left a comment

Choose a reason for hiding this comment

ghost commented Jan 25, 2022 •

edited by ghost

Loading