Merge pull request #68 from pullhub:pause

Implement pauseRequest, and bugfixes

src/adapter.ts

@@ -110,6 +110,7 @@ export class perlDebuggerConnection extends EventEmitter {
 	public padwalkerVersion: string;
 	public commandRunning: string = '';
 	public isRemote: boolean = false;
+	public debuggerPid?: number;
 
 	private filename?: string;
 	private rootPath?: string;
@@ -508,6 +509,15 @@ export class perlDebuggerConnection extends EventEmitter {
 		// Initial data from debugger
 		this.logData('', data.slice(0, data.length-2));
 
+		// While `runInTerminal` is supposed to give us the pid of the
+		// spawned `perl -d` process, that does not work very well as of
+		// 2019-02. Instead we ask Perl for the host process id. Note
+		// that the value is meaningful only if `this.isRemote` is false.
+		// For local processes the pid is needed to send `SIGINT` to the
+		// debugger, which is supposed to break into the debugger and
+		// used to implement the `pauseRequest`.
+		this.debuggerPid = parseInt(await this.getExpressionValue('$$'));
+
 		try {
 			// Get the version just after
 			this.perlVersion = await this.getPerlVersion();

src/perlDebug.ts

@@ -4,6 +4,7 @@ import {
 	Logger, logger,
 	DebugSession, LoggingDebugSession,
 	InitializedEvent, TerminatedEvent, StoppedEvent, BreakpointEvent, OutputEvent, Event,
+	ContinuedEvent,
 	Thread, StackFrame, Scope, Source, Handles, Breakpoint, Variable,
 	LoadedSourceEvent
 } from 'vscode-debugadapter';
@@ -267,9 +268,33 @@ export class PerlDebugSession extends LoggingDebugSession {
 
 
 	protected pauseRequest(response: DebugProtocol.PauseResponse, args: DebugProtocol.PauseArguments): void {
-		this.sendEvent(new OutputEvent(`ERR>pause not implemented\n`));
-		this.sendResponse(response);
-		this.sendEvent(new StoppedEvent("breakpoint", PerlDebugSession.THREAD_ID));
+
+		if (this.perlDebugger.isRemote) {
+			response.success = false;
+			response.body = {
+				error: {
+					message: 'Cannot send SIGINT to debugger on remote system'
+				}
+			};
+			this.sendResponse(response);
+
+		} else {
+
+			// Send SIGINT to the `perl -d` process on the local system.
+			process.kill(this.perlDebugger.debuggerPid, 'SIGINT');
+			this.sendResponse(response);
+
+			// TODO(bh): It is not clear if we are supposed to also send a
+			// `StoppedEvent` and if we are, what the logic for that ought
+			// to be. Basically, whenever we see the `DB<N>` prompt from
+			// the debugger we are most probably stopped. That's the same
+			// for all protocol functions though, which suggests that ought
+			// to be handled centrally someplace, and not individually for
+			// each request. As it is, in any case, it does not seem to
+			// make a difference in vscode whether send one here or not.
+
+		}
+
 	}
 
 
@@ -319,6 +344,7 @@ export class PerlDebugSession extends LoggingDebugSession {
 			.catch(err => {
 				const [ error = err ] = err.errors || [];
 				this.sendEvent(new OutputEvent(`ERR>setFunctionBreakPointsRequest error: ${error.message}\n`));
+				response.success = false;
 				this.sendResponse(response);
 			});
 	}
@@ -347,6 +373,7 @@ export class PerlDebugSession extends LoggingDebugSession {
 			.catch((err) => {
 				const [ error = err ] = err.errors || [];
 				this.sendEvent(new OutputEvent(`ERR>setVariableRequest error: ${error.message}\n`));
+				response.success = false;
 				this.sendResponse(response);
 			});
 	}
@@ -378,6 +405,7 @@ export class PerlDebugSession extends LoggingDebugSession {
 					this.sendEvent(new OutputEvent(`ERR>StepOut error: ${error.message}\n`));
 					this.sendEvent(new TerminatedEvent());
 				}
+				response.success = false;
 				this.sendResponse(response);
 			});
 	}
@@ -409,6 +437,7 @@ export class PerlDebugSession extends LoggingDebugSession {
 					this.sendEvent(new OutputEvent(`ERR>StepIn error: ${error.message}\n`));
 					this.sendEvent(new TerminatedEvent());
 				}
+				response.success = false;
 				this.sendResponse(response);
 			});
 	}
@@ -433,7 +462,10 @@ export class PerlDebugSession extends LoggingDebugSession {
 	protected restartRequest(response: DebugProtocol.RestartResponse, args: DebugProtocol.RestartArguments): void {
 		this.restartRequestAsync(response, args)
 			.then(res => this.sendResponse(res))
-			.catch(err => this.sendResponse(response));
+			.catch(err => {
+				response.success = false;
+				this.sendResponse(response);
+			});
 	}
 
 	/**
@@ -495,7 +527,10 @@ export class PerlDebugSession extends LoggingDebugSession {
 	protected setBreakPointsRequest(response: DebugProtocol.SetBreakpointsResponse, args: DebugProtocol.SetBreakpointsArguments): void {
 		this.setBreakPointsRequestAsync(response, args)
 			.then(res => this.sendResponse(res))
-			.catch(err => this.sendResponse(response));
+			.catch(err => {
+				response.success = false;
+				this.sendResponse(response)
+			});
 	}
 
 	/**
@@ -525,6 +560,7 @@ export class PerlDebugSession extends LoggingDebugSession {
 					this.sendEvent(new OutputEvent(`ERR>Next error: ${error.message}\n`));
 					this.sendEvent(new TerminatedEvent());
 				}
+				response.success = false;
 				this.sendResponse(response);
 			});
 	}
@@ -534,6 +570,20 @@ export class PerlDebugSession extends LoggingDebugSession {
 	 * Continue
 	 */
 	protected continueRequest(response: DebugProtocol.ContinueResponse, args: DebugProtocol.ContinueArguments): void {
+
+		// NOTE(bh): "Please note: a debug adapter is not expected to
+		// send this event in response to a request that implies that
+		// execution continues, e.g. ‘launch’ or ‘continue’." -- but in
+		// our case sending the `c` command to the debugger is never
+		// acknowledged by the debugger, we cannot tell if it succeeded
+		// and the promise below is resolved only once the debugger has
+		// halted execution of the debuggee again. Without a response to
+		// the `continueRequest`, vscode does not offer users a `pause`
+		// button, so without sending this event, we cannot pause from
+		// the debug user interface. So we send the event...
+
+		this.sendEvent(new ContinuedEvent(PerlDebugSession.THREAD_ID));
+
 		this.perlDebugger.request('c')
 			.then((res) => {
 				if (res.ln) {
@@ -555,6 +605,8 @@ export class PerlDebugSession extends LoggingDebugSession {
 					this.sendEvent(new OutputEvent(`ERR>Continue error: ${error.message}\n`));
 					this.sendEvent(new TerminatedEvent());
 				}
+
+				response.success = false;
 				this.sendResponse(response);
 			});
 	}
@@ -622,6 +674,7 @@ export class PerlDebugSession extends LoggingDebugSession {
 				}
 			})
 			.catch(() => {
+				response.success = false;
 				this.sendResponse(response);
 			});
 	}
@@ -801,6 +854,7 @@ export class PerlDebugSession extends LoggingDebugSession {
 			.catch(err => {
 				const [ error = err ] = err.errors || [];
 				this.sendEvent(new OutputEvent(`--->Trace error...${error.message}\n`));
+				response.success = false;
 				response.body = {
 					stackFrames: [],
 					totalFrames: 0
@@ -853,6 +907,7 @@ export class PerlDebugSession extends LoggingDebugSession {
 
 				const [ error = err ] = err.errors || [];
 				this.sendEvent(new OutputEvent(`--->Loaded sources request error...${error.message}\n`));
+				response.success = false;
 				response.body = {
 					sources: [
 					]
@@ -894,9 +949,10 @@ export class PerlDebugSession extends LoggingDebugSession {
 
 				const [ error = err ] = err.errors || [];
 				this.sendEvent(new OutputEvent(`--->Source request error...${error.message}\n`));
+				response.success = false;
 				response.body = {
 					content: `# error`,
-					mimeType: `text/vnd.vscode-perl-debug.error`
+					mimeType: `text/vnd.vscode-perl-debug.error`,
 				};
 				this.sendResponse(response);
 

src/tests/adapter.test.ts

@@ -26,6 +26,7 @@ describe('Perl debug Adapter', () => {
 	const FILE_BROKEN_CODE = 'broken_code.pl';
 	const FILE_PRINT_ARGUMENTS = 'print_arguments.pl';
 	const FILE_FAST_TEST_PL = 'fast_test.pl';
+	const FILE_LONG_RUNNING_PL = 'long_running.pl';
 
 	const PERL_DEBUG_LOG = 'perl_debugger.log';
 
@@ -150,6 +151,53 @@ describe('Perl debug Adapter', () => {
 		});
 	});
 
+	describe('pause', () => {
+
+		it('should be able to pause programs', async () => {
+			const PROGRAM = Path.join(DATA_ROOT, FILE_LONG_RUNNING_PL);
+
+			// NOTE(bh): This test is probably expected to fail when test
+			// and adapter run in the same process?
+
+			await dc.launch(Configuration({
+				program: PROGRAM,
+				stopOnEntry: true
+			}));
+
+			dc.continueRequest({
+				threadId: undefined
+			});
+
+			// NOTE(bh): Perl's built-in `sleep` function only supports
+			// integer resolution sleeps, so this test is a bit slow.
+
+			await new Promise(resolve => setTimeout(resolve, 2200));
+
+			await dc.pauseRequest({
+				threadId: undefined,
+			});
+
+			// The evaluate request can only succeed within a few seconds
+			// if the debuggee is actually stopped, otherwise the debugger
+			// would not take our request in time because it's on the same
+			// thread as the debuggee. If things do not go according to
+			// plan and the debuggee keeps running, it will be killed by
+			// the test runner due to a timeout since the script runs for
+			// around 30 seconds, longer than the timeout.
+
+			const result = await dc.evaluateRequest({
+				context: 'repl',
+				expression: 'p $_'
+			});
+
+			assert.ok(
+				parseInt(result.body.result) > 3,
+				'must have gone at least twice through the loop'
+			);
+
+		});
+	});
+
 	// xxx: Need to figure out this test
 	// hint: It might be a missing "stop" event - is the application run?
 	describe.skip('setBreakpoints', () => {

src/tests/data/long_running.pl

@@ -0,0 +1,3 @@
+for (1 .. 30) {
+  sleep 1;
+}