Merge pull request #81403 from dgozman/process-queue

Process debug adapter messages in separate tasks; see #33822, #79196
microsoft · Dec 12, 2019 · d0c8e7a · d0c8e7a
2 parents 5c569d1 + bacbfd2
commit d0c8e7a
Show file tree

Hide file tree

Showing 4 changed files with 126 additions and 24 deletions.
diff --git a/src/vs/workbench/contrib/debug/browser/debugSession.ts b/src/vs/workbench/contrib/debug/browser/debugSession.ts
@@ -691,6 +691,11 @@ export class DebugSession implements IDebugSession {
 		}
 	}
 
+	initializeForTest(raw: RawDebugSession): void {
+		this.raw = raw;
+		this.registerListeners();
+	}
+
 	//---- private
 
 	private registerListeners(): void {

diff --git a/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts b/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts
@@ -5,7 +5,7 @@
 
 import { Emitter, Event } from 'vs/base/common/event';
 import { IDebugAdapter } from 'vs/workbench/contrib/debug/common/debug';
-import { timeout } from 'vs/base/common/async';
+import { timeout, Queue } from 'vs/base/common/async';
 
 /**
  * Abstract implementation of the low level API for a debug adapter.
@@ -18,6 +18,7 @@ export abstract class AbstractDebugAdapter implements IDebugAdapter {
 	private requestCallback: ((request: DebugProtocol.Request) => void) | undefined;
 	private eventCallback: ((request: DebugProtocol.Event) => void) | undefined;
 	private messageCallback: ((message: DebugProtocol.ProtocolMessage) => void) | undefined;
+	private readonly queue = new Queue();
 	protected readonly _onError = new Emitter<Error>();
 	protected readonly _onExit = new Emitter<number | null>();
 
@@ -108,26 +109,33 @@ export abstract class AbstractDebugAdapter implements IDebugAdapter {
 			this.messageCallback(message);
 		}
 		else {
-			switch (message.type) {
-				case 'event':
-					if (this.eventCallback) {
-						this.eventCallback(<DebugProtocol.Event>message);
-					}
-					break;
-				case 'request':
-					if (this.requestCallback) {
-						this.requestCallback(<DebugProtocol.Request>message);
-					}
-					break;
-				case 'response':
-					const response = <DebugProtocol.Response>message;
-					const clb = this.pendingRequests.get(response.request_seq);
-					if (clb) {
-						this.pendingRequests.delete(response.request_seq);
-						clb(response);
-					}
-					break;
-			}
+			this.queue.queue(() => {
+				switch (message.type) {
+					case 'event':
+						if (this.eventCallback) {
+							this.eventCallback(<DebugProtocol.Event>message);
+						}
+						break;
+					case 'request':
+						if (this.requestCallback) {
+							this.requestCallback(<DebugProtocol.Request>message);
+						}
+						break;
+					case 'response':
+						const response = <DebugProtocol.Response>message;
+						const clb = this.pendingRequests.get(response.request_seq);
+						if (clb) {
+							this.pendingRequests.delete(response.request_seq);
+							clb(response);
+						}
+						break;
+				}
+
+				// Artificially queueing protocol messages guarantees that any microtasks for
+				// previous message finish before next message is processed. This is essential
+				// to guarantee ordering when using promises anywhere along the call path.
+				return timeout(0);
+			});
 		}
 	}
 
@@ -164,6 +172,6 @@ export abstract class AbstractDebugAdapter implements IDebugAdapter {
 	}
 
 	dispose(): void {
-		// noop
+		this.queue.dispose();
 	}
 }
diff --git a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts b/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts
@@ -8,12 +8,14 @@ import { URI as uri } from 'vs/base/common/uri';
 import severity from 'vs/base/common/severity';
 import { DebugModel, Expression, StackFrame, Thread } from 'vs/workbench/contrib/debug/common/debugModel';
 import * as sinon from 'sinon';
-import { MockRawSession } from 'vs/workbench/contrib/debug/test/common/mockDebug';
+import { MockRawSession, MockDebugAdapter } from 'vs/workbench/contrib/debug/test/common/mockDebug';
 import { Source } from 'vs/workbench/contrib/debug/common/debugSource';
 import { DebugSession } from 'vs/workbench/contrib/debug/browser/debugSession';
-import { SimpleReplElement, RawObjectReplElement, ReplEvaluationInput, ReplModel } from 'vs/workbench/contrib/debug/common/replModel';
+import { SimpleReplElement, RawObjectReplElement, ReplEvaluationInput, ReplModel, ReplEvaluationResult } from 'vs/workbench/contrib/debug/common/replModel';
 import { IBreakpointUpdateData, IDebugSessionOptions } from 'vs/workbench/contrib/debug/common/debug';
 import { NullOpenerService } from 'vs/platform/opener/common/opener';
+import { RawDebugSession } from 'vs/workbench/contrib/debug/browser/rawDebugSession';
+import { timeout } from 'vs/base/common/async';
 
 function createMockSession(model: DebugModel, name = 'mockSession', options?: IDebugSessionOptions): DebugSession {
 	return new DebugSession({ resolved: { name, type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, options, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, NullOpenerService);
@@ -548,4 +550,26 @@ suite('Debug - Model', () => {
 		assert.equal(grandChild.getReplElements().length, 2);
 		assert.equal(child3.getReplElements().length, 1);
 	});
+
+	test('repl ordering', async () => {
+		const session = createMockSession(model);
+		model.addSession(session);
+
+		const adapter = new MockDebugAdapter();
+		const raw = new RawDebugSession(adapter, undefined!, undefined!, undefined!, undefined!, undefined!);
+		session.initializeForTest(raw);
+
+		await session.addReplExpression(undefined, 'before.1');
+		assert.equal(session.getReplElements().length, 3);
+		assert.equal((<ReplEvaluationInput>session.getReplElements()[0]).value, 'before.1');
+		assert.equal((<SimpleReplElement>session.getReplElements()[1]).value, 'before.1');
+		assert.equal((<ReplEvaluationResult>session.getReplElements()[2]).value, '=before.1');
+
+		await session.addReplExpression(undefined, 'after.2');
+		await timeout(0);
+		assert.equal(session.getReplElements().length, 6);
+		assert.equal((<ReplEvaluationInput>session.getReplElements()[3]).value, 'after.2');
+		assert.equal((<ReplEvaluationResult>session.getReplElements()[4]).value, '=after.2');
+		assert.equal((<SimpleReplElement>session.getReplElements()[5]).value, 'after.2');
+	});
 });
diff --git a/src/vs/workbench/contrib/debug/test/common/mockDebug.ts b/src/vs/workbench/contrib/debug/test/common/mockDebug.ts
@@ -11,6 +11,7 @@ import { ILaunch, IDebugService, State, IDebugSession, IConfigurationManager, IS
 import { Source } from 'vs/workbench/contrib/debug/common/debugSource';
 import { CompletionItem } from 'vs/editor/common/modes';
 import Severity from 'vs/base/common/severity';
+import { AbstractDebugAdapter } from 'vs/workbench/contrib/debug/common/abstractDebugAdapter';
 
 export class MockDebugService implements IDebugService {
 
@@ -464,3 +465,67 @@ export class MockRawSession {
 
 	public readonly onDidStop: Event<DebugProtocol.StoppedEvent> = null!;
 }
+
+export class MockDebugAdapter extends AbstractDebugAdapter {
+	private seq = 0;
+
+	startSession(): Promise<void> {
+		return Promise.resolve();
+	}
+
+	stopSession(): Promise<void> {
+		return Promise.resolve();
+	}
+
+	sendMessage(message: DebugProtocol.ProtocolMessage): void {
+		setTimeout(() => {
+			if (message.type === 'request') {
+				const request = message as DebugProtocol.Request;
+				switch (request.command) {
+					case 'evaluate':
+						this.evaluate(request, request.arguments);
+						return;
+				}
+				this.sendResponseBody(request, {});
+				return;
+			}
+		}, 0);
+	}
+
+	sendResponseBody(request: DebugProtocol.Request, body: any) {
+		const response: DebugProtocol.Response = {
+			seq: ++this.seq,
+			type: 'response',
+			request_seq: request.seq,
+			command: request.command,
+			success: true,
+			body
+		};
+		this.acceptMessage(response);
+	}
+
+	sendEventBody(event: string, body: any) {
+		const response: DebugProtocol.Event = {
+			seq: ++this.seq,
+			type: 'event',
+			event,
+			body
+		};
+		this.acceptMessage(response);
+	}
+
+	evaluate(request: DebugProtocol.Request, args: DebugProtocol.EvaluateArguments) {
+		if (args.expression.indexOf('before.') === 0) {
+			this.sendEventBody('output', { output: args.expression });
+		}
+
+		this.sendResponseBody(request, {
+			result: '=' + args.expression,
+			variablesReference: 0
+		});
+
+		if (args.expression.indexOf('after.') === 0) {
+			this.sendEventBody('output', { output: args.expression });
+		}
+	}
+}