Merge pull request #14214 from Microsoft/CancellationChecksForLowPriorityTasks

Adding cancellation token checks for lower priority tasks (navbar & outlining spans)

Author: jramsay

src/harness/unittests/tsserverProjectSystem.ts

@@ -176,11 +176,11 @@ namespace ts.projectSystem {
         }
     };
 
-    export function createSession(host: server.ServerHost, typingsInstaller?: server.ITypingsInstaller, projectServiceEventHandler?: server.ProjectServiceEventHandler, cancellationToken?: server.ServerCancellationToken) {
+    export function createSession(host: server.ServerHost, typingsInstaller?: server.ITypingsInstaller, projectServiceEventHandler?: server.ProjectServiceEventHandler, cancellationToken?: server.ServerCancellationToken, throttleWaitMilliseconds?: number) {
         if (typingsInstaller === undefined) {
             typingsInstaller = new TestTypingsInstaller("/a/data/", /*throttleLimit*/5, host);
         }
-        return new TestSession(host, cancellationToken || server.nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ projectServiceEventHandler !== undefined, projectServiceEventHandler);
+        return new TestSession(host, cancellationToken || server.nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ projectServiceEventHandler !== undefined, projectServiceEventHandler, throttleWaitMilliseconds);
     }
 
     export interface CreateProjectServiceParameters {
@@ -547,6 +547,49 @@ namespace ts.projectSystem {
         readonly getEnvironmentVariable = notImplemented;
     }
 
+    /**
+      * Test server cancellation token used to mock host token cancellation requests.
+      * The cancelAfterRequest constructor param specifies how many isCancellationRequested() calls
+      * should be made before canceling the token. The id of the request to cancel should be set with
+      * setRequestToCancel();
+      */
+    export class TestServerCancellationToken implements server.ServerCancellationToken {
+        private currentId = -1;
+        private requestToCancel = -1;
+        private isCancellationRequestedCount = 0;
+
+        constructor(private cancelAfterRequest = 0) {
+        }
+
+        setRequest(requestId: number) {
+            this.currentId = requestId;
+        }
+
+        setRequestToCancel(requestId: number) {
+            this.resetToken();
+            this.requestToCancel = requestId;
+        }
+
+        resetRequest(requestId: number) {
+            assert.equal(requestId, this.currentId, "unexpected request id in cancellation");
+            this.currentId = undefined;
+        }
+
+        isCancellationRequested() {
+            this.isCancellationRequestedCount++;
+            // If the request id is the request to cancel and isCancellationRequestedCount
+            // has been met then cancel the request. Ex: cancel the request if it is a
+            // nav bar request & isCancellationRequested() has already been called three times.
+            return this.requestToCancel === this.currentId && this.isCancellationRequestedCount >= this.cancelAfterRequest;
+        }
+
+        resetToken() {
+            this.currentId = -1;
+            this.isCancellationRequestedCount = 0;
+            this.requestToCancel = -1;
+        }
+    }
+
     export function makeSessionRequest<T>(command: string, args: T) {
         const newRequest: protocol.Request = {
             seq: 0,
@@ -3384,6 +3427,7 @@ namespace ts.projectSystem {
                 },
                 resetRequest: noop
             };
+
             const session = createSession(host, /*typingsInstaller*/ undefined, /*projectServiceEventHandler*/ undefined, cancellationToken);
 
             expectedRequestId = session.getNextSeq();
@@ -3422,22 +3466,7 @@ namespace ts.projectSystem {
                 })
             };
 
-            let requestToCancel = -1;
-            const cancellationToken: server.ServerCancellationToken = (function(){
-                let currentId: number;
-                return <server.ServerCancellationToken>{
-                    setRequest(requestId) {
-                        currentId = requestId;
-                    },
-                    resetRequest(requestId) {
-                        assert.equal(requestId, currentId, "unexpected request id in cancellation");
-                        currentId = undefined;
-                    },
-                    isCancellationRequested() {
-                        return requestToCancel === currentId;
-                    }
-                };
-            })();
+            const cancellationToken = new TestServerCancellationToken();
             const host = createServerHost([f1, config]);
             const session = createSession(host, /*typingsInstaller*/ undefined, () => {}, cancellationToken);
             {
@@ -3472,13 +3501,13 @@ namespace ts.projectSystem {
                 host.clearOutput();
 
                 // cancel previously issued Geterr
-                requestToCancel = getErrId;
+                cancellationToken.setRequestToCancel(getErrId);
                 host.runQueuedTimeoutCallbacks();
 
                 assert.equal(host.getOutput().length, 1, "expect 1 message");
                 verifyRequestCompleted(getErrId, 0);
 
-                requestToCancel = -1;
+                cancellationToken.resetToken();
             }
             {
                 const getErrId = session.getNextSeq();
@@ -3495,12 +3524,12 @@ namespace ts.projectSystem {
                 assert.equal(e1.event, "syntaxDiag");
                 host.clearOutput();
 
-                requestToCancel = getErrId;
+                cancellationToken.setRequestToCancel(getErrId);
                 host.runQueuedImmediateCallbacks();
                 assert.equal(host.getOutput().length, 1, "expect 1 message");
                 verifyRequestCompleted(getErrId, 0);
 
-                requestToCancel = -1;
+                cancellationToken.resetToken();
             }
             {
                 const getErrId = session.getNextSeq();
@@ -3523,7 +3552,7 @@ namespace ts.projectSystem {
                 assert.equal(e2.event, "semanticDiag");
                 verifyRequestCompleted(getErrId, 1);
 
-                requestToCancel = -1;
+                cancellationToken.resetToken();
             }
             {
                 const getErr1 = session.getNextSeq();
@@ -3558,6 +3587,68 @@ namespace ts.projectSystem {
                 return JSON.parse(server.extractMessage(host.getOutput()[n]));
             }
         });
+        it("Lower priority tasks are cancellable", () => {
+            const f1 = {
+                path: "/a/app.ts",
+                content: `{ let x = 1; } var foo = "foo"; var bar = "bar"; var fooBar = "fooBar";`
+            };
+            const config = {
+                path: "/a/tsconfig.json",
+                content: JSON.stringify({
+                    compilerOptions: {}
+                })
+            };
+            const cancellationToken = new TestServerCancellationToken(/*cancelAfterRequest*/ 3);
+            const host = createServerHost([f1, config]);
+            const session = createSession(host, /*typingsInstaller*/ undefined, () => { }, cancellationToken, /*throttleWaitMilliseconds*/ 0);
+            {
+                session.executeCommandSeq(<protocol.OpenRequest>{
+                    command: "open",
+                    arguments: { file: f1.path }
+                });
+
+                // send navbar request (normal priority)
+                session.executeCommandSeq(<protocol.NavBarRequest>{
+                    command: "navbar",
+                    arguments: { file: f1.path }
+                });
+
+                // ensure the nav bar request can be canceled
+                verifyExecuteCommandSeqIsCancellable(<protocol.NavBarRequest>{
+                    command: "navbar",
+                    arguments: { file: f1.path }
+                });
+
+                // send outlining spans request (normal priority)
+                session.executeCommandSeq(<protocol.OutliningSpansRequest>{
+                    command: "outliningSpans",
+                    arguments: { file: f1.path }
+                });
+
+                // ensure the outlining spans request can be canceled
+                verifyExecuteCommandSeqIsCancellable(<protocol.OutliningSpansRequest>{
+                    command: "outliningSpans",
+                    arguments: { file: f1.path }
+                });
+            }
+
+            function verifyExecuteCommandSeqIsCancellable<T extends server.protocol.Request>(request: Partial<T>) {
+                // Set the next request to be cancellable
+                // The cancellation token will cancel the request the third time
+                // isCancellationRequested() is called.
+                cancellationToken.setRequestToCancel(session.getNextSeq());
+                let operationCanceledExceptionThrown = false;
+
+                try {
+                    session.executeCommandSeq(request);
+                }
+                catch (e) {
+                    assert(e instanceof OperationCanceledException);
+                    operationCanceledExceptionThrown = true;
+                }
+                assert(operationCanceledExceptionThrown, "Operation Canceled Exception not thrown for request: " + JSON.stringify(request));
+            }
+        });
     });
 
     describe("occurence highlight on string", () => {

src/server/editorServices.ts

@@ -271,7 +271,8 @@ namespace ts.server {
             public readonly cancellationToken: HostCancellationToken,
             public readonly useSingleInferredProject: boolean,
             readonly typingsInstaller: ITypingsInstaller = nullTypingsInstaller,
-            private readonly eventHandler?: ProjectServiceEventHandler) {
+            private readonly eventHandler?: ProjectServiceEventHandler,
+            public readonly throttleWaitMilliseconds?: number) {
 
             Debug.assert(!!host.createHash, "'ServerHost.createHash' is required for ProjectService");
 

src/server/lsHost.ts

@@ -16,6 +16,7 @@ namespace ts.server {
         readonly realpath?: (path: string) => string;
 
         constructor(private readonly host: ServerHost, private readonly project: Project, private readonly cancellationToken: HostCancellationToken) {
+            this.cancellationToken = new ThrottledCancellationToken(cancellationToken, project.projectService.throttleWaitMilliseconds);
             this.getCanonicalFileName = ts.createGetCanonicalFileName(this.host.useCaseSensitiveFileNames);
 
             if (host.trace) {

src/server/session.ts

@@ -337,7 +337,8 @@ namespace ts.server {
             private hrtime: (start?: number[]) => number[],
             protected logger: Logger,
             protected readonly canUseEvents: boolean,
-            eventHandler?: ProjectServiceEventHandler) {
+            eventHandler?: ProjectServiceEventHandler,
+            private readonly throttleWaitMilliseconds?: number) {
 
             this.eventHander = canUseEvents
                 ? eventHandler || (event => this.defaultEventHandler(event))
@@ -352,7 +353,7 @@ namespace ts.server {
                 isCancellationRequested: () => cancellationToken.isCancellationRequested()
             };
             this.errorCheck = new MultistepOperation(multistepOperationHost);
-            this.projectService = new ProjectService(host, logger, cancellationToken, useSingleInferredProject, typingsInstaller, this.eventHander);
+            this.projectService = new ProjectService(host, logger, cancellationToken, useSingleInferredProject, typingsInstaller, this.eventHander, this.throttleWaitMilliseconds);
             this.gcTimer = new GcTimer(host, /*delay*/ 7000, logger);
         }
 

src/services/navigationBar.ts

@@ -2,6 +2,36 @@
 
 /* @internal */
 namespace ts.NavigationBar {
+    /**
+     * Matches all whitespace characters in a string. Eg:
+     *
+     * "app.
+     *
+     * onactivated"
+     *
+     * matches because of the newline, whereas
+     *
+     * "app.onactivated"
+     *
+     * does not match.
+     */
+    const whiteSpaceRegex = /\s+/g;
+
+    // Keep sourceFile handy so we don't have to search for it every time we need to call `getText`.
+    let curCancellationToken: CancellationToken;
+    let curSourceFile: SourceFile;
+
+    /**
+     * For performance, we keep navigation bar parents on a stack rather than passing them through each recursion.
+     * `parent` is the current parent and is *not* stored in parentsStack.
+     * `startNode` sets a new parent and `endNode` returns to the previous parent.
+     */
+    let parentsStack: NavigationBarNode[] = [];
+    let parent: NavigationBarNode;
+
+    // NavigationBarItem requires an array, but will not mutate it, so just give it this for performance.
+    let emptyChildItemArray: NavigationBarItem[] = [];
+
     /**
      * Represents a navigation bar item and its children.
      * The returned NavigationBarItem is more complicated and doesn't include 'parent', so we use these to do work before converting.
@@ -14,22 +44,36 @@ namespace ts.NavigationBar {
         indent: number; // # of parents
     }
 
-    export function getNavigationBarItems(sourceFile: SourceFile): NavigationBarItem[] {
+    export function getNavigationBarItems(sourceFile: SourceFile, cancellationToken: CancellationToken): NavigationBarItem[] {
+        curCancellationToken = cancellationToken;
         curSourceFile = sourceFile;
-        const result = map(topLevelItems(rootNavigationBarNode(sourceFile)), convertToTopLevelItem);
-        curSourceFile = undefined;
-        return result;
+        try {
+            return map(topLevelItems(rootNavigationBarNode(sourceFile)), convertToTopLevelItem);
+        }
+        finally {
+            reset();
+        }
     }
 
-    export function getNavigationTree(sourceFile: SourceFile): NavigationTree {
+    export function getNavigationTree(sourceFile: SourceFile, cancellationToken: CancellationToken): NavigationTree {
+        curCancellationToken = cancellationToken;
         curSourceFile = sourceFile;
-        const result = convertToTree(rootNavigationBarNode(sourceFile));
+        try {
+            return convertToTree(rootNavigationBarNode(sourceFile));
+        }
+        finally {
+            reset();
+        }
+    }
+
+    function reset() {
         curSourceFile = undefined;
-        return result;
+        curCancellationToken = undefined;
+        parentsStack = [];
+        parent = undefined;
+        emptyChildItemArray = [];
     }
 
-    // Keep sourceFile handy so we don't have to search for it every time we need to call `getText`.
-    let curSourceFile: SourceFile;
     function nodeText(node: Node): string {
         return node.getText(curSourceFile);
     }
@@ -47,14 +91,6 @@ namespace ts.NavigationBar {
         }
     }
 
-    /*
-    For performance, we keep navigation bar parents on a stack rather than passing them through each recursion.
-    `parent` is the current parent and is *not* stored in parentsStack.
-    `startNode` sets a new parent and `endNode` returns to the previous parent.
-    */
-    const parentsStack: NavigationBarNode[] = [];
-    let parent: NavigationBarNode;
-
     function rootNavigationBarNode(sourceFile: SourceFile): NavigationBarNode {
         Debug.assert(!parentsStack.length);
         const root: NavigationBarNode = { node: sourceFile, additionalNodes: undefined, parent: undefined, children: undefined, indent: 0 };
@@ -111,6 +147,8 @@ namespace ts.NavigationBar {
 
     /** Look for navigation bar items in node's subtree, adding them to the current `parent`. */
     function addChildrenRecursively(node: Node): void {
+        curCancellationToken.throwIfCancellationRequested();
+
         if (!node || isToken(node)) {
             return;
         }
@@ -487,9 +525,6 @@ namespace ts.NavigationBar {
         }
     }
 
-    // NavigationBarItem requires an array, but will not mutate it, so just give it this for performance.
-    const emptyChildItemArray: NavigationBarItem[] = [];
-
     function convertToTree(n: NavigationBarNode): NavigationTree {
         return {
             text: getItemName(n.node),
@@ -610,19 +645,4 @@ namespace ts.NavigationBar {
     function isFunctionOrClassExpression(node: Node): boolean {
         return node.kind === SyntaxKind.FunctionExpression || node.kind === SyntaxKind.ArrowFunction || node.kind === SyntaxKind.ClassExpression;
     }
-
-    /**
-     * Matches all whitespace characters in a string. Eg:
-     *
-     * "app.
-     *
-     * onactivated"
-     *
-     * matches because of the newline, whereas
-     *
-     * "app.onactivated"
-     *
-     * does not match.
-     */
-    const whiteSpaceRegex = /\s+/g;
 }