Fix debug file not getting deleted

package.json

@@ -46,8 +46,8 @@
   "devDependencies": {
     "@types/fs-extra": "^5.0.4",
     "@types/mocha": "^5.2.5",
-    "@types/node": "^6.14.0",
-    "fs-extra": "^7.0.0",
+    "@types/node": "^8.10.43",
+    "fs-extra": "^7.0.1",
     "tslint": "^5.11.0",
     "typescript": "^3.1.3",
     "vscode": "^1.1.26"

src/debugAdapter/goDebug.ts

@@ -5,13 +5,18 @@
 
 import * as path from 'path';
 import * as os from 'os';
+import * as fs from 'fs';
+import * as util from 'util';
 import { DebugProtocol } from 'vscode-debugprotocol';
 import { DebugSession, InitializedEvent, TerminatedEvent, ThreadEvent, StoppedEvent, OutputEvent, Thread, StackFrame, Scope, Source, Handles, LoggingDebugSession, Logger, logger } from 'vscode-debugadapter';
 import { existsSync, lstatSync } from 'fs';
 import { basename, dirname, extname } from 'path';
 import { spawn, ChildProcess, execSync, spawnSync, execFile } from 'child_process';
 import { Client, RPCConnection } from 'json-rpc2';
-import { parseEnvFile, getBinPathWithPreferredGopath, resolveHomeDir, getInferredGopath, getCurrentGoWorkspaceFromGOPATH, envPath, fixDriveCasingInWindows } from '../goPath';
+import { parseEnvFile, getBinPathWithPreferredGopath, getInferredGopath, getCurrentGoWorkspaceFromGOPATH, envPath, fixDriveCasingInWindows } from '../goPath';
+
+const fsAccess = util.promisify(fs.access);
+const fsUnlink = util.promisify(fs.unlink);
 
 // This enum should stay in sync with https://golang.org/pkg/reflect/#Kind
 
@@ -260,6 +265,7 @@ function normalizePath(filePath: string) {
 class Delve {
 	program: string;
 	remotePath: string;
+	localDebugeePath: string | undefined;
 	debugProcess: ChildProcess;
 	loadConfig: LoadConfig;
 	connection: Promise<RPCConnection>;
@@ -357,7 +363,7 @@ class Delve {
 							logError('Process exiting with code: ' + code);
 							if (this.onclose) { this.onclose(code); }
 						});
-						this.debugProcess.on('error', function(err) {
+						this.debugProcess.on('error', (err) => {
 							reject(err);
 						});
 						resolve();
@@ -391,7 +397,7 @@ class Delve {
 				return reject(`Cannot find Delve debugger. Install from https://github.com/derekparker/delve & ensure it is in your Go tools path, "GOPATH/bin" or "PATH".`);
 			}
 
-			let currentGOWorkspace = getCurrentGoWorkspaceFromGOPATH(env['GOPATH'], dirname);
+			const currentGOWorkspace = getCurrentGoWorkspaceFromGOPATH(env['GOPATH'], dirname);
 			let dlvArgs = [mode || 'debug'];
 			if (mode === 'exec') {
 				dlvArgs = dlvArgs.concat([program]);
@@ -436,6 +442,7 @@ class Delve {
 				env,
 			});
 
+			this.localDebugeePath = this.getLocalDebugeePath(launchArgs.output);
 			function connectClient(port: number, host: string) {
 				// Add a slight delay to avoid issues on Linux with
 				// Delve failing calls made shortly after connection.
@@ -465,7 +472,7 @@ class Delve {
 				logError('Process exiting with code: ' + code);
 				if (this.onclose) { this.onclose(code); }
 			});
-			this.debugProcess.on('error', function(err) {
+			this.debugProcess.on('error', (err) => {
 				reject(err);
 			});
 		});
@@ -479,71 +486,85 @@ class Delve {
 		});
 	}
 
-	callPromise<T>(command: string, args: any[]): Thenable<T> {
+	public callPromise<T>(command: string, args: any[]): Thenable<T> {
 		return new Promise<T>((resolve, reject) => {
 			this.connection.then(conn => {
-				conn.call<T>('RPCServer.' + command, args, (err, res) => {
-					if (err) return reject(err);
-					resolve(res);
+				conn.call<T>(`RPCServer.${command}`, args, (err, res) => {
+					return err ? reject(err) : resolve(res);
 				});
 			}, err => {
 				reject(err);
 			});
 		});
 	}
 
-	close(): Thenable<void> {
+	/**
+	 * Closing a debugging session follows different approaches for local vs remote delve process.
+	 *
+	 * For local process, since the extension starts the delve process, the extension should close it as well.
+	 * To gracefully clean up the assets created by delve, we send the Detach request with kill option set to true.
+	 *
+	 * For remote process, since the extension doesnt start the delve process, we only detach from it without killing it.
+	 *
+	 * The only way to detach from delve when it is running a program is to send a Halt request first.
+	 * Since the Halt request might sometimes take too long to complete, we have a timer in place to forcefully kill
+	 * the debug process and clean up the assets in case of local debugging
+	 */
+	public close(): Thenable<void> {
 		if (this.noDebug) {
 			// delve isn't running so no need to halt
 			return Promise.resolve();
 		}
 		log('HaltRequest');
 
-		return new Promise(resolve => {
-			let timeoutToken: NodeJS.Timer;
-			if (this.debugProcess) {
-				timeoutToken = setTimeout(() => {
-					log('Killing debug process manually as we could not halt and detach delve in time');
-					killTree(this.debugProcess.pid);
-					resolve();
-				}, 1000);
-			}
-
-			this.callPromise('Command', [{ name: 'halt' }]).then(() => {
-				if (timeoutToken) {
-					clearTimeout(timeoutToken);
-				}
+		const isLocalDebugging: boolean = !!this.debugProcess;
+		const forceCleanup = async () => {
+			killTree(this.debugProcess.pid);
+			await removeFile(this.localDebugeePath);
+		};
+		return new Promise(async resolve => {
+			const timeoutToken: NodeJS.Timer = isLocalDebugging && setTimeout(async () => {
+				log('Killing debug process manually as we could not halt delve in time');
+				await forceCleanup();
+				resolve();
+			}, 1000);
+
+			let haltErrMsg;
+			try {
+				await this.callPromise('Command', [{ name: 'halt' }]);
+			} catch (err) {
 				log('HaltResponse');
-				if (!this.debugProcess) {
-					log('RestartRequest');
-					return this.callPromise('Restart', this.isApiV1 ? [] : [{ position: '', resetArgs: false, newArgs: [] }])
-						.then(null, err => {
-							log('RestartResponse');
-							logError(`Failed to restart - ${(err || '').toString()}`);
-						})
-						.then(() => resolve());
-				} else {
-					log('DetachRequest');
-					return this.callPromise('Detach', [this.isApiV1 ? true : { Kill: true }])
-						.then(null, err => {
-							log('DetachResponse');
-							logError(`Killing debug process manually as we failed to detach - ${(err || '').toString()}`);
-							killTree(this.debugProcess.pid);
-						})
-						.then(() => resolve());
-				}
-			}, err => {
-				const errMsg = err ? err.toString() : '';
-				log('Failed to halt - ' + errMsg.toString());
-				if (errMsg.endsWith('has exited with status 0')) {
-					if (timeoutToken) {
-						clearTimeout(timeoutToken);
-					}
-					return resolve();
+				haltErrMsg = err ? err.toString() : '';
+				log(`Failed to halt - ${haltErrMsg}`);
+			}
+			clearTimeout(timeoutToken);
+
+			const targetHasExited: boolean = haltErrMsg && haltErrMsg.endsWith('has exited with status 0');
+			const shouldDetach: boolean = !haltErrMsg || targetHasExited;
+			let shouldForceClean: boolean = !shouldDetach && isLocalDebugging;
+			if (shouldDetach) {
+				log('DetachRequest');
+				try {
+					await this.callPromise('Detach', [this.isApiV1 ? true : { Kill: isLocalDebugging }]);
+				} catch (err) {
+					log('DetachResponse');
+					logError(`Failed to detach - ${(err.toString() || '')}`);
+					shouldForceClean = isLocalDebugging;
 				}
-			});
+			}
+			if (shouldForceClean) {
+				await forceCleanup();
+			}
+			return resolve();
 		});
 	}
+
+	private getLocalDebugeePath(output: string | undefined): string {
+		const configOutput = output || 'debug';
+		return path.isAbsolute(configOutput)
+			? configOutput
+			: path.resolve(this.program, configOutput);
+	}
 }
 
 class GoDebugSession extends LoggingDebugSession {
@@ -1406,4 +1427,17 @@ function killTree(processId: number): void {
 	}
 }
 
+async function removeFile(filePath: string): Promise<void> {
+	try {
+		const fileExists = await fsAccess(filePath)
+			.then(() => true)
+			.catch(() => false);
+		if (filePath && fileExists) {
+			await fsUnlink(filePath);
+		}
+	} catch (e) {
+		logError(`Potentially failed remove file: ${filePath} - ${e.toString() || ''}`);
+	}
+}
+
 DebugSession.run(GoDebugSession);

test/go.test.ts

@@ -544,22 +544,18 @@ It returns the number of bytes written and any write error encountered.
 	test('Test Outline document symbols', (done) => {
 		const uri = vscode.Uri.file(path.join(fixturePath, 'outlineTest', 'test.go'));
 		vscode.workspace.openTextDocument(uri).then(document => {
-			new GoDocumentSymbolProvider().provideDocumentSymbols(document, null).then(symbols => {
-				const groupedSymbolNames = symbols.reduce((map: any, symbol) => {
-					map[symbol.kind] = (map[symbol.kind] || []).concat([symbol.name]);
-					return map;
-				}, {});
-
-				const packageNames = groupedSymbolNames[vscode.SymbolKind.Package];
-				const variableNames = groupedSymbolNames[vscode.SymbolKind.Variable];
-				const functionNames = groupedSymbolNames[vscode.SymbolKind.Function];
-				const structs = groupedSymbolNames[vscode.SymbolKind.Struct];
-				assert.equal(packageNames[0], 'main');
-				assert.equal(variableNames, undefined);
-				assert.equal(functionNames[0], 'print');
-				assert.equal(functionNames[1], 'main');
+			new GoDocumentSymbolProvider().provideDocumentSymbols(document, null).then(outlines => {
+				const packages = outlines.filter(x => x.kind === vscode.SymbolKind.Package);
+				const variables = outlines[0].children.filter((x: any) => x.kind === vscode.SymbolKind.Variable);
+				const functions = outlines[0].children.filter((x: any) => x.kind === vscode.SymbolKind.Function);
+				const structs = outlines[0].children.filter((x: any) => x.kind === vscode.SymbolKind.Struct);
+
+				assert.equal(packages[0].name, 'main');
+				assert.equal(variables.length, 0);
+				assert.equal(functions[0].name, 'print');
+				assert.equal(functions[1].name, 'main');
 				assert.equal(structs.length, 1);
-				assert.equal(structs[0], 'foo');
+				assert.equal(structs[0].name, 'foo');
 			});
 		}).then(() => done(), done);
 	});