Skip to content

Commit

Permalink
Fix debugger hang on restart (#3846)
Browse files Browse the repository at this point in the history
* telemetry check for trigger, also include trigger and folder in obj for debugger

* update origin telemetry

* add enum for debug origin

* pass in from command info and cleanup

* Update cmakeFileApiDriver.ts take out import

* move to 1.19 changelog

---------

Co-authored-by: Garrett Campbell <[email protected]>
  • Loading branch information
qarni and gcampbell-msft authored Jul 9, 2024
1 parent b7951a7 commit 934c4d0
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 72 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Bug Fixes:

- Fix issue where `cmake.preferredGenerators` wasn't falling back to the next entry when the first entry didn't exist [#2709](https://github.com/microsoft/vscode-cmake-tools/issues/2709)
- Potential fix for attempting to load a non-variants file as a variants file and throwing a parse exception [#3727](https://github.com/microsoft/vscode-cmake-tools/issues/3727)
- Fix issue where `Configure with CMake Debugger` fails on restart because the previously used pipe to CMake Debugger is no longer available. [#3582](https://github.com/microsoft/vscode-cmake-tools/issues/3582)

## 1.18.43

Expand Down
10 changes: 8 additions & 2 deletions src/cmakeProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1636,8 +1636,14 @@ export class CMakeProject {
.then(async chosen => {
if (chosen) {
if (chosen.title === yesButtonTitle) {
await this.configureInternal(ConfigureTrigger.configureFailedConfigureWithDebuggerButton, extraArgs, ConfigureType.NormalWithDebugger, {
pipeName: getDebuggerPipeName()
await vscode.debug.startDebugging(undefined, {
name: localize("cmake.debug.name", "CMake Debugger"),
request: "launch",
type: "cmake",
cmakeDebugType: "configure",
pipeName: getDebuggerPipeName(),
trigger: ConfigureTrigger.configureFailedConfigureWithDebuggerButton,
fromCommand: true
});
} else if (chosen.title === doNotShowAgainTitle) {
await cmakeConfiguration.update(showDebuggerConfigurationString, false, vscode.ConfigurationTarget.Global);
Expand Down
5 changes: 4 additions & 1 deletion src/debug/cmakeDebuggerTelemetry.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import * as telemetry from "@cmt/telemetry";

export const originatedFromLaunchConfiguration: string = "launchConfiguration";
export enum DebugOrigin {
originatedFromLaunchConfiguration = "launchConfiguration",
originatedFromCommand = "command"
}

export function logCMakeDebuggerTelemetry(origin: string, debugType: string) {
telemetry.logEvent("cmakeDebugger", {
Expand Down
100 changes: 54 additions & 46 deletions src/debug/debugAdapterNamedPipeServerDescriptorFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { executeScriptWithDebugger } from "./debuggerScriptDriver";
import * as logging from '../logging';
import * as nls from "vscode-nls";
import { fs } from "../pr";
import { logCMakeDebuggerTelemetry, originatedFromLaunchConfiguration } from "./cmakeDebuggerTelemetry";
import { DebugOrigin, logCMakeDebuggerTelemetry} from "./cmakeDebuggerTelemetry";
import { ConfigureTrigger } from "@cmt/cmakeProject";

nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })();
const localize: nls.LocalizeFunc = nls.loadMessageBundle();
Expand All @@ -17,60 +18,67 @@ export class DebugAdapterNamedPipeServerDescriptorFactory implements vscode.Debu
async createDebugAdapterDescriptor(session: vscode.DebugSession, _executable: vscode.DebugAdapterExecutable | undefined): Promise<vscode.ProviderResult<vscode.DebugAdapterDescriptor>> {
const pipeName =
session.configuration.pipeName ?? getDebuggerPipeName();
const origin =
session.configuration.fromCommand ? DebugOrigin.originatedFromCommand : DebugOrigin.originatedFromLaunchConfiguration;

// undocumented configuration field that lets us know if the session is being invoked from a command
// This should only be used from inside the extension from a command that invokes the debugger.
if (!session.configuration.fromCommand) {
const debuggerInformation: DebuggerInformation = {
pipeName,
dapLog: session.configuration.dapLog,
debuggerIsReady: () => undefined
};
const debuggerInformation: DebuggerInformation = {
pipeName,
dapLog: session.configuration.dapLog,
debuggerIsReady: () => undefined
};

const cmakeDebugType: "configure" | "script" | "external" = session.configuration.cmakeDebugType;
if (cmakeDebugType === "configure" || cmakeDebugType === "script") {
const promise = new Promise<void>((resolve) => {
debuggerInformation.debuggerIsReady = resolve;
});
const cmakeDebugType: "configure" | "script" | "external" = session.configuration.cmakeDebugType;
if (cmakeDebugType === "configure" || cmakeDebugType === "script") {
const promise = new Promise<void>((resolve) => {
debuggerInformation.debuggerIsReady = resolve;
});

if (cmakeDebugType === "script") {
const script = session.configuration.scriptPath;
if (!fs.existsSync(script)) {
throw new Error(localize("cmake.debug.scriptPath.does.not.exist", "The script path, \"{0}\", could not be found.", script));
if (cmakeDebugType === "script") {
const script = session.configuration.scriptPath;
if (!fs.existsSync(script)) {
throw new Error(localize("cmake.debug.scriptPath.does.not.exist", "The script path, \"{0}\", could not be found.", script));
}
const args: string[] = session.configuration.scriptArgs ?? [];
const env = new Map<string, string>(session.configuration.scriptEnv?.map((e: {name: string; value: string}) => [e.name, e.value])) ?? new Map();
logCMakeDebuggerTelemetry(origin, cmakeDebugType);
void executeScriptWithDebugger(script, args, env, debuggerInformation);
} else {
logCMakeDebuggerTelemetry(origin, cmakeDebugType);

if (session.configuration.trigger && !Object.values(ConfigureTrigger).includes(session.configuration.trigger)) {
session.configuration.trigger = undefined;
}

if (session.configuration.clean) {
if (session.configuration.configureAll) {
void extensionManager?.cleanConfigureAllWithDebuggerInternal(
debuggerInformation,
session.configuration.trigger
);
} else {
void extensionManager?.cleanConfigureWithDebuggerInternal(
debuggerInformation,
session.configuration.folder
);
}
const args: string[] = session.configuration.scriptArgs ?? [];
const env = new Map<string, string>(session.configuration.scriptEnv?.map((e: {name: string; value: string}) => [e.name, e.value])) ?? new Map();
logCMakeDebuggerTelemetry(originatedFromLaunchConfiguration, cmakeDebugType);
void executeScriptWithDebugger(script, args, env, debuggerInformation);
} else {
logCMakeDebuggerTelemetry(originatedFromLaunchConfiguration, cmakeDebugType);
if (session.configuration.clean) {
if (session.configuration.configureAll) {
void extensionManager?.cleanConfigureAllWithDebuggerInternal(
debuggerInformation
);
} else {
void extensionManager?.cleanConfigureWithDebuggerInternal(
debuggerInformation
);
}
if (session.configuration.configureAll) {
void extensionManager?.configureAllWithDebuggerInternal(
debuggerInformation,
session.configuration.trigger
);
} else {
if (session.configuration.configureAll) {
void extensionManager?.configureAllWithDebuggerInternal(
debuggerInformation
);
} else {
void extensionManager?.configureWithDebuggerInternal(
debuggerInformation
);
}
void extensionManager?.configureWithDebuggerInternal(
debuggerInformation,
session.configuration.folder, session.configuration.sourceDir, session.configuration.trigger
);
}
}

await promise;
} else if (cmakeDebugType === "external") {
logCMakeDebuggerTelemetry(originatedFromLaunchConfiguration, cmakeDebugType);
}

await promise;
} else if (cmakeDebugType === "external") {
logCMakeDebuggerTelemetry(origin, cmakeDebugType);
}

logger.info(localize('debugger.create.descriptor', 'Connecting debugger on named pipe: \"{0}\"', pipeName));
Expand Down
2 changes: 1 addition & 1 deletion src/debug/debuggerConfigureDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as vscode from "vscode";
export interface DebuggerInformation {
pipeName: string;
dapLog?: string;
debuggerIsReady?(): void;
debuggerIsReady(): void;
}

export function getDebuggerPipeName(): string {
Expand Down
21 changes: 3 additions & 18 deletions src/drivers/cmakeFileApiDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import * as nls from 'vscode-nls';
import { DebuggerInformation } from '@cmt/debug/debuggerConfigureDriver';
import { CMakeOutputConsumer, StateMessage } from '@cmt/diagnostics/cmake';
import { ConfigureTrigger } from '@cmt/cmakeProject';
import { logCMakeDebuggerTelemetry } from '@cmt/debug/cmakeDebuggerTelemetry';
import { onConfigureSettingsChange } from '@cmt/ui/util';

nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })();
Expand Down Expand Up @@ -290,23 +289,9 @@ export class CMakeFileApiDriver extends CMakeDriver {
await new Promise(resolve => setTimeout(resolve, 50));
}

// if there isn't a `debuggerIsReady` callback provided, this means that this invocation was
// started by a command, rather than by a launch configuration, and the debug session will start from here.
if (debuggerInformation.debuggerIsReady) {
// This cmake debug invocation came from a launch configuration. All telemetry is handled in the createDebugAdapterDescriptor handler.
debuggerInformation.debuggerIsReady();
} else {
const cmakeDebugType = "configure";
logCMakeDebuggerTelemetry(trigger ?? "", cmakeDebugType);
await vscode.debug.startDebugging(undefined, {
name: localize("cmake.debug.name", "CMake Debugger"),
request: "launch",
type: "cmake",
cmakeDebugType,
pipeName: debuggerInformation.pipeName,
fromCommand: true
});
}
// This cmake debug invocation was started from a startDebugging command.
// All telemetry is handled in the createDebugAdapterDescriptor handler.
debuggerInformation.debuggerIsReady();
}
}

Expand Down
42 changes: 38 additions & 4 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1286,7 +1286,15 @@ export class ExtensionManager implements vscode.Disposable {
}

cleanConfigureWithDebugger(folder?: vscode.WorkspaceFolder) {
return this.cleanConfigureWithDebuggerInternal({pipeName: getDebuggerPipeName()}, folder);
return vscode.debug.startDebugging(undefined, {
name: localize("cmake.debug.name", "CMake Debugger"),
request: "launch",
type: "cmake",
cmakeDebugType: "configure",
pipeName: getDebuggerPipeName(),
folder,
fromCommand: true
});
}

cleanConfigureWithDebuggerInternal(debuggerInformation: DebuggerInformation, folder?: vscode.WorkspaceFolder) {
Expand All @@ -1300,7 +1308,15 @@ export class ExtensionManager implements vscode.Disposable {
}

cleanConfigureAllWithDebugger(trigger?: ConfigureTrigger) {
return this.cleanConfigureAllWithDebuggerInternal({pipeName: getDebuggerPipeName()}, trigger);
return vscode.debug.startDebugging(undefined, {
name: localize("cmake.debug.name", "CMake Debugger"),
request: "launch",
type: "cmake",
cmakeDebugType: "configure",
pipeName: getDebuggerPipeName(),
trigger,
fromCommand: true
});
}

cleanConfigureAllWithDebuggerInternal(debuggerInformation: DebuggerInformation, trigger?: ConfigureTrigger) {
Expand All @@ -1315,7 +1331,17 @@ export class ExtensionManager implements vscode.Disposable {
}

configureWithDebugger(folder?: vscode.WorkspaceFolder, sourceDir?: string, trigger?: ConfigureTrigger) {
return this.configureWithDebuggerInternal({pipeName: getDebuggerPipeName()}, folder, undefined, sourceDir, trigger);
return vscode.debug.startDebugging(undefined, {
name: localize("cmake.debug.name", "CMake Debugger"),
request: "launch",
type: "cmake",
cmakeDebugType: "configure",
pipeName: getDebuggerPipeName(),
folder,
sourceDir,
trigger,
fromCommand: true
});
}

configureWithDebuggerInternal(debuggerInformation: DebuggerInformation, folder?: vscode.WorkspaceFolder, showCommandOnly?: boolean, sourceDir?: string, trigger?: ConfigureTrigger) {
Expand All @@ -1333,7 +1359,15 @@ export class ExtensionManager implements vscode.Disposable {
}

configureAllWithDebugger(trigger?: ConfigureTrigger) {
return this.configureAllWithDebuggerInternal({pipeName: getDebuggerPipeName()}, trigger);
return vscode.debug.startDebugging(undefined, {
name: localize("cmake.debug.name", "CMake Debugger"),
request: "launch",
type: "cmake",
cmakeDebugType: "configure",
pipeName: getDebuggerPipeName(),
trigger,
fromCommand: true
});
}

configureAllWithDebuggerInternal(debuggerInformation: DebuggerInformation, trigger?: ConfigureTrigger) {
Expand Down

0 comments on commit 934c4d0

Please sign in to comment.