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

DSPLaunch hangs when used with Dart Debug Adapter #262

Closed
sebthom opened this issue Oct 20, 2022 · 6 comments · Fixed by #263
Closed

DSPLaunch hangs when used with Dart Debug Adapter #262

sebthom opened this issue Oct 20, 2022 · 6 comments · Fixed by #263
Labels
bug Something isn't working

Comments

@sebthom
Copy link
Member

sebthom commented Oct 20, 2022

I am trying to integrate the dart debug adapter with lsp4j/lsp4e, however after the first view messages the communication hangs and I have no idea why. I doubt that it's LSP4E, but a more underlying problem, so I opened the issue here.

This is the communication trace:

{
  "jsonrpc": "2.0",
  "id": 1,
  "method": "initialize",
  "params": {
    "clientID": "lsp4e.debug",
    "adapterID": "adapterId",
    "linesStartAt1": true,
    "columnsStartAt1": true,
    "pathFormat": "path",
    "supportsVariableType": true,
    "supportsVariablePaging": true,
    "supportsRunInTerminalRequest": true
  }
}
{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "supportsConfigurationDoneRequest": true,
    "supportsConditionalBreakpoints": true,
    "supportsEvaluateForHovers": true,
    "exceptionBreakpointFilters": [
      {
        "filter": "All",
        "label": "All Exceptions",
        "default": false
      },
      {
        "filter": "Unhandled",
        "label": "Uncaught Exceptions",
        "default": true
      }
    ],
    "supportsRestartRequest": false,
    "supportsDelayedStackTraceLoading": true,
    "supportsLogPoints": true,
    "supportsTerminateRequest": true,
    "supportsClipboardContext": true
  }
}
{
  "jsonrpc": "2.0",
  "id": 2,
  "method": "launch",
  "params": {
    "args": [],
    "cwd": "D:\\workspaces\\projects\\dart-hotreloader",
    "vmAdditionalArgs": [],
    "program": "D:\\workspaces\\projects\\dart-hotreloader\\lib\\main.dart",
    "noDebug": false
  }
}
{
  "jsonrpc": "2.0",
  "method": "initialized",
  "params": null
}

That is what I see in the UI:
image

The Debug Adapter can be launched manually using the command dart debug_adapter. This is how I launch it:

var debuggerOpts = new TreeBuilder<String>() 
   .put("cwd", workdir.toString())
   .put("program", dartMainFilePath.toString())
   .put("args", programArgs)
   .put("vmAdditionalArgs", vmArgs)
   .getMap();

try {
   var builder = new DSPLaunchDelegateLaunchBuilder(config, ILaunchManager.DEBUG_MODE, launch, monitor);
   builder.setLaunchDebugAdapter(dartSDK.getDartExecutable().toString(), List.of("debug_adapter"));
   builder.setMonitorDebugAdapter(config.getAttribute(DSPPlugin.ATTR_DSP_MONITOR_DEBUG_ADAPTER, true));
   builder.setDspParameters(debuggerOpts);
   new LaunchDebugConfig().launch(builder);
} catch (final CoreException ex) {
   Dialogs.showStatus("Failed to start debug session", Dart4EPlugin.status().createError(ex), true);
}

This is the documentation for the Dart DAP support https://github.com/dart-lang/sdk/blob/main/pkg/dds/tool/dap/README.md

Any thoughts/ideas are very much appreciated.

@jonahgraham
Copy link
Contributor

Do you want to commit your LSP4E extending code somewhere, I can check it out and see if I can track it down.

@sebthom
Copy link
Member Author

sebthom commented Oct 20, 2022

@jonahgraham thanks for the offer. I am in an early phase, but the plugin is buildable/runnable https://github.com/dart4e/dart4e/blob/main/plugin/src/main/java/org/dart4e/launch/LaunchConfigLauncher.java

@jonahgraham jonahgraham transferred this issue from eclipse-lsp4j/lsp4j Oct 21, 2022
@jonahgraham
Copy link
Contributor

The issue here is in LSP4E and I have transferred to LSP4E repo.

A while ago an attempt at fixing an NPE fundamentally changed the behaviour of the debug client. When the client receives the initialized event it must start sending out the breakpoints, followed by (if supported) the configuration done. The entire startup is then supposed to be marked complete when both the configurationDone response is received and the launch request response is received.

The current code:

future = CompletableFuture.allOf(future, initialized).thenRun(() -> {

is waiting for:

  • future which is the Future for the launch/attach response
  • initialized which is the Future for the initialized event

Then once both of those are received, it is sending the breakpoints followed by the configurationDone request:

future = CompletableFuture.allOf(future, initialized).thenRun(() -> {
monitor.worked(10);
}).thenCompose(v -> {
monitor.worked(10);
monitor.subTask("Sending breakpoints");
breakpointManager = new DSPBreakpointManager(getBreakpointManager(), getDebugProtocolServer(),
capabilities);
return breakpointManager.initialize();
}).thenCompose(v -> {
monitor.worked(30);
monitor.subTask("Sending configuration done");
if (Boolean.TRUE.equals(capabilities.getSupportsConfigurationDoneRequest())) {
return getDebugProtocolServer().configurationDone(new ConfigurationDoneArguments());
}
return CompletableFuture.completedFuture(null);
});

However the client needs to start sending breakpoints once initialized is received. The debug adapter can send initialized event anytime after the initialize request and may not send a response to launch/attach request until as late as after configurationDone response.

The picture in microsoft/vscode#4902 (comment) is a better picture showing the parallel nature of the debug client when talking to the adapter than this pic in the DAP spec

Unfortunately #66 moved a bracket around to address an NPE, but that wasn't the correct fix. Reverting #66 makes the dart debug adapter work properly again. However that reintroduces the potential NPE. For that I will open another issue as it requires its own long discussion.

@sebthom
Copy link
Member Author

sebthom commented Oct 21, 2022

@mickaelistria Now that this got fixed. I would be very interested in getting a new LSP4E release that includes this patch as well as #256. WDYT, how soon would that be possible?

@mickaelistria
Copy link
Contributor

Let's wait for #265 before considering a release.

@jonahgraham
Copy link
Contributor

Let's wait for #265 before considering a release.

It is now merged.

@jonahgraham jonahgraham added the bug Something isn't working label Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants