Skip to content

Commit e8f68c1

Browse files
committed
Adds MultiCancellationToken
This is a cancellation token that cancels when any of its constituent cancellation tokens are cancelled. This token is used to fix a bug in Find Definitions. Previously, when clicking `CTRL` (or `CMD` on macs) inside a source file in an archive and hovering over a token, this will automatically invoke the definitions finder (in preparation for navigating to the definition). The only way to cancel is to move down to the message popup and click cancel there. However, this is a bug. What _should_ happen is that if a user moves their mouse away from the token, the operation should cancel. The underlying problem is that the extension was only listening to the cancellation token from inside `getLocationsForUriString` the cancellation token used by the Language Server protocol to cancel operations in flight was being ignored. This fix will ensure we are listening to _both_ cancellation tokens and cancel the query if either are cancelled.
1 parent 3489c26 commit e8f68c1

File tree

2 files changed

+51
-12
lines changed

2 files changed

+51
-12
lines changed
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { CancellationToken, Disposable } from "vscode";
2+
3+
/**
4+
* A cancellation token that cancels when any of its constituent
5+
* cancellation tokens are cancelled.
6+
*/
7+
export class MultiCancellationToken implements CancellationToken {
8+
private readonly tokens: CancellationToken[];
9+
10+
constructor(...tokens: CancellationToken[]) {
11+
this.tokens = tokens;
12+
}
13+
14+
get isCancellationRequested(): boolean {
15+
return this.tokens.some((t) => t.isCancellationRequested);
16+
}
17+
18+
onCancellationRequested<T>(listener: (e: T) => any): Disposable {
19+
this.tokens.forEach((t) => t.onCancellationRequested(listener));
20+
return {
21+
dispose: () => {
22+
this.tokens.forEach((t) =>
23+
t.onCancellationRequested(listener).dispose(),
24+
);
25+
},
26+
};
27+
}
28+
}

extensions/ql-vscode/src/language-support/contextual/template-provider.ts

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,15 @@ import {
3636
import { CoreCompletedQuery, QueryRunner } from "../../query-server";
3737
import { AstBuilder } from "../ast-viewer/ast-builder";
3838
import { qlpackOfDatabase } from "../../local-queries";
39+
import { MultiCancellationToken } from "../../common/multi-cancellation-token";
3940

4041
/**
4142
* Runs templated CodeQL queries to find definitions in
4243
* source-language files. We may eventually want to find a way to
4344
* generalize this to other custom queries, e.g. showing dataflow to
4445
* or from a selected identifier.
4546
*/
47+
4648
export class TemplateQueryDefinitionProvider implements DefinitionProvider {
4749
private cache: CachedOperation<LocationLink[]>;
4850

@@ -60,11 +62,11 @@ export class TemplateQueryDefinitionProvider implements DefinitionProvider {
6062
async provideDefinition(
6163
document: TextDocument,
6264
position: Position,
63-
_token: CancellationToken,
65+
token: CancellationToken,
6466
): Promise<LocationLink[]> {
6567
const fileLinks = this.shouldUseCache()
66-
? await this.cache.get(document.uri.toString())
67-
: await this.getDefinitions(document.uri.toString());
68+
? await this.cache.get(document.uri.toString(), token)
69+
: await this.getDefinitions(document.uri.toString(), token);
6870

6971
const locLinks: LocationLink[] = [];
7072
for (const link of fileLinks) {
@@ -79,9 +81,13 @@ export class TemplateQueryDefinitionProvider implements DefinitionProvider {
7981
return !(isCanary() && NO_CACHE_CONTEXTUAL_QUERIES.getValue<boolean>());
8082
}
8183

82-
private async getDefinitions(uriString: string): Promise<LocationLink[]> {
84+
private async getDefinitions(
85+
uriString: string,
86+
token: CancellationToken,
87+
): Promise<LocationLink[]> {
8388
return withProgress(
84-
async (progress, token) => {
89+
async (progress, tokenInner) => {
90+
const multiToken = new MultiCancellationToken(token, tokenInner);
8591
return getLocationsForUriString(
8692
this.cli,
8793
this.qs,
@@ -90,7 +96,7 @@ export class TemplateQueryDefinitionProvider implements DefinitionProvider {
9096
KeyType.DefinitionQuery,
9197
this.queryStorageDir,
9298
progress,
93-
token,
99+
multiToken,
94100
(src, _dest) => src === uriString,
95101
);
96102
},
@@ -126,11 +132,11 @@ export class TemplateQueryReferenceProvider implements ReferenceProvider {
126132
document: TextDocument,
127133
position: Position,
128134
_context: ReferenceContext,
129-
_token: CancellationToken,
135+
token: CancellationToken,
130136
): Promise<Location[]> {
131137
const fileLinks = this.shouldUseCache()
132-
? await this.cache.get(document.uri.toString())
133-
: await this.getReferences(document.uri.toString());
138+
? await this.cache.get(document.uri.toString(), token)
139+
: await this.getReferences(document.uri.toString(), token);
134140

135141
const locLinks: Location[] = [];
136142
for (const link of fileLinks) {
@@ -148,9 +154,14 @@ export class TemplateQueryReferenceProvider implements ReferenceProvider {
148154
return !(isCanary() && NO_CACHE_CONTEXTUAL_QUERIES.getValue<boolean>());
149155
}
150156

151-
private async getReferences(uriString: string): Promise<FullLocationLink[]> {
157+
private async getReferences(
158+
uriString: string,
159+
token: CancellationToken,
160+
): Promise<FullLocationLink[]> {
152161
return withProgress(
153-
async (progress, token) => {
162+
async (progress, tokenInner) => {
163+
const multiToken = new MultiCancellationToken(token, tokenInner);
164+
154165
return getLocationsForUriString(
155166
this.cli,
156167
this.qs,
@@ -159,7 +170,7 @@ export class TemplateQueryReferenceProvider implements ReferenceProvider {
159170
KeyType.DefinitionQuery,
160171
this.queryStorageDir,
161172
progress,
162-
token,
173+
multiToken,
163174
(src, _dest) => src === uriString,
164175
);
165176
},

0 commit comments

Comments
 (0)