Skip to content

Commit

Permalink
Address some race issues with preview and cancellation (#4403)
Browse files Browse the repository at this point in the history
* Address some race issues with preview and cancellation
  • Loading branch information
Colengms authored and sean-mcmanus committed Oct 8, 2019
1 parent a8537c4 commit e043c13
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 76 deletions.
75 changes: 33 additions & 42 deletions Extension/src/LanguageServer/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ export class DefaultClient implements Client {
}
referencesRequestPending = true;
// Register a single-fire handler for the reply.
let resultCallback: refs.ReferencesResultCallback = (result: refs.ReferencesResult, referencesCanceledWhilePreviewing: boolean) => {
let resultCallback: refs.ReferencesResultCallback = (result: refs.ReferencesResult) => {
referencesRequestPending = false;
let locations: vscode.Location[] = [];
if (result) {
Expand All @@ -645,9 +645,7 @@ export class DefaultClient implements Client {
});
}
// If references were canceled while in a preview state, there is not an outstanding promise.
if (!referencesCanceledWhilePreviewing) {
resolve(locations);
}
resolve(locations);
if (referencesPendingCancellations.length > 0) {
while (referencesPendingCancellations.length > 1) {
let pendingCancel: ReferencesCancellationState = referencesPendingCancellations[0];
Expand All @@ -659,13 +657,23 @@ export class DefaultClient implements Client {
pendingCancel.callback();
}
};
if (this.client.references.lastResults) {
let lastResults: refs.ReferencesResult = this.client.references.lastResults;
this.client.references.lastResults = null;
resultCallback(lastResults, true);
} else {
if (!this.client.references.referencesRefreshPending) {
this.client.references.setResultsCallback(resultCallback);
this.client.references.startFindAllReferences(params);
} else {
// We are responding to a refresh (preview or final result)
this.client.references.referencesRefreshPending = false;
if (this.client.references.lastResults) {
// This is a final result
let lastResults: refs.ReferencesResult = this.client.references.lastResults;
this.client.references.lastResults = null;
resultCallback(lastResults);
} else {
// This is a preview (2nd or later preview)
this.client.references.referencesRequestPending = true;
this.client.references.setResultsCallback(resultCallback);
this.client.languageClient.sendNotification(RequestReferencesNotification, false);
}
}
});
token.onCancellationRequested(e => {
Expand All @@ -675,7 +683,7 @@ export class DefaultClient implements Client {
});
};

if (referencesRequestPending || (this.client.references.symbolSearchInProgress && !this.client.references.referencesViewFindPending)) {
if (referencesRequestPending || (this.client.references.symbolSearchInProgress && !this.client.references.referencesRefreshPending)) {
let cancelling: boolean = referencesPendingCancellations.length > 0;
referencesPendingCancellations.push({ reject: () => {
// Complete with nothing instead of rejecting, to avoid an error message from VS Code
Expand All @@ -685,9 +693,6 @@ export class DefaultClient implements Client {
if (!cancelling) {
renamePending = false;
this.client.references.referencesCanceled = true;
if (!referencesRequestPending) {
this.client.references.referencesCanceledWhilePreviewing = true;
}
this.client.languageClient.sendNotification(CancelReferencesNotification);
this.client.references.closeRenameUI();
}
Expand Down Expand Up @@ -735,7 +740,7 @@ export class DefaultClient implements Client {
return;
}
referencesRequestPending = true;
this.client.references.setResultsCallback((referencesResult: refs.ReferencesResult, referencesCanceledWhilePreviewing: boolean) => {
this.client.references.setResultsCallback((referencesResult: refs.ReferencesResult) => {
referencesRequestPending = false;
--renameRequestsPending;
let workspaceEdit: vscode.WorkspaceEdit = new vscode.WorkspaceEdit();
Expand Down Expand Up @@ -780,9 +785,6 @@ export class DefaultClient implements Client {
}, callback });
if (!cancelling) {
this.client.references.referencesCanceled = true;
if (!referencesRequestPending) {
this.client.references.referencesCanceledWhilePreviewing = true;
}
this.client.languageClient.sendNotification(CancelReferencesNotification);
this.client.references.closeRenameUI();
}
Expand Down Expand Up @@ -2210,36 +2212,25 @@ export class DefaultClient implements Client {
if (!cancelling) {
this.references.UpdateProgressUICounter(this.model.referencesCommandMode.Value);
if (this.ReferencesCommandMode === refs.ReferencesCommandMode.Find) {
this.sendRequestReferences();
if (!this.references.referencesRequestPending) {
if (this.references.referencesRequestHasOccurred) {
// References are not usable if a references request is pending,
// So after the initial request, we don't send a 2nd references request until the next request occurs.
if (!this.references.referencesRefreshPending) {
this.references.referencesRefreshPending = true;
vscode.commands.executeCommand("references-view.refresh");
}
} else {
this.references.referencesRequestHasOccurred = true;
this.references.referencesRequestPending = true;
this.languageClient.sendNotification(RequestReferencesNotification, false);
}
}
}
}
});
}

public sendRequestReferences(): void {
switch (this.model.referencesCommandMode.Value) {
case refs.ReferencesCommandMode.None:
break;
case refs.ReferencesCommandMode.Peek:
case refs.ReferencesCommandMode.Rename:
this.languageClient.sendNotification(RequestReferencesNotification, true);
break;
default:
if (this.references.referencesRequestHasOccurred) {
// References are not usable if a references request is pending,
// So after the initial request, we don't send a 2nd references request until the next request occurs.
if (!this.references.referencesViewFindPending) {
this.references.referencesViewFindPending = true;
vscode.commands.executeCommand("references-view.refresh");
}
} else {
this.languageClient.sendNotification(RequestReferencesNotification, false);
this.references.referencesRequestHasOccurred = true;
}
break;
}
}

public cancelReferences(): void {
referencesParams = null;
renamePending = false;
Expand Down
66 changes: 32 additions & 34 deletions Extension/src/LanguageServer/references.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export interface ReferencesResult {
isFinished: boolean;
}

export type ReferencesResultCallback = (result: ReferencesResult, referencesCanceledWhilePreviewing: boolean) => void;
export type ReferencesResultCallback = (result: ReferencesResult) => void;

export interface ReferencesResultMessage {
referencesResult: ReferencesResult;
Expand Down Expand Up @@ -135,11 +135,12 @@ export class ReferencesManager {
private referencesPrevProgressIncrement: number;
private referencesPrevProgressMessage: string;
public referencesRequestHasOccurred: boolean = false;
public referencesViewFindPending: boolean = false;
private referencesFinished: boolean = false;
public referencesRequestPending: boolean = false;
public referencesRefreshPending: boolean = false;
private referencesDelayProgress: NodeJS.Timeout;
private referencesProgressOptions: vscode.ProgressOptions;
public referencesCanceled: boolean = false;
public referencesCanceledWhilePreviewing: boolean = false;
private referencesStartedWhileTagParsing: boolean;
private referencesProgressMethod: (progress: vscode.Progress<{
message?: string;
Expand Down Expand Up @@ -336,42 +337,36 @@ export class ReferencesManager {
}

public startRename(params: RenameParams): void {
this.lastResults = null;
this.referencesFinished = false;
this.referencesRequestHasOccurred = false;
this.referencesRequestPending = false;
if (this.referencesCanceled) {
// Request was canceled before the initial request was sent, so cancel message was already sent.
// Deliver empty canceled result.

// Need to reset these before we call the callback, as the callback my trigger another request
// and we need to ensure these values are already reset before that happens.
this.referencesRequestHasOccurred = false;
this.referencesCanceled = false;
this.referencesCanceledWhilePreviewing = false;

this.resultsCallback(null, true);
this.resultsCallback(null);
} else {
this.client.sendRenameNofication(params);
}
}

public startFindAllReferences(params: FindAllReferencesParams): void {
this.lastResults = null;
this.referencesFinished = false;
this.referencesRequestHasOccurred = false;
this.referencesRequestPending = false;
if (this.referencesCanceled) {
// Request was canceled before the initial request was sent, so cancel message was already sent.
// Deliver empty canceled result.

// Need to reset these before we call the callback, as the callback my trigger another request
// and we need to ensure these values are already reset before that happens.
this.referencesRequestHasOccurred = false;
this.referencesCanceled = false;
this.referencesCanceledWhilePreviewing = false;

this.resultsCallback(null, true);
this.resultsCallback(null);
} else {
this.client.sendFindAllReferencesNotification(params);
}
}

public processResults(referencesResult: ReferencesResult): void {
if (this.referencesFinished) {
return;
}
this.initializeViews();
this.referencesViewFindPending = false;
this.clearViews();

if (this.client.ReferencesCommandMode === ReferencesCommandMode.Peek && !this.referencesChannel) {
Expand All @@ -396,12 +391,10 @@ export class ReferencesManager {

// Need to reset these before we call the callback, as the callback my trigger another request
// and we need to ensure these values are already reset before that happens.
let referencesCanceledWhilePreviewing: boolean = this.referencesCanceledWhilePreviewing;
let referencesRequestHasOccurred: boolean = this.referencesRequestHasOccurred;
let referencesRequestPending: boolean = this.referencesRequestPending;
let referencesCanceled: boolean = this.referencesCanceled;
this.referencesRequestHasOccurred = false;
this.referencesRequestPending = false;
this.referencesCanceled = false;
this.referencesCanceledWhilePreviewing = false;

let currentReferenceCommandMode: ReferencesCommandMode = this.client.ReferencesCommandMode;

Expand All @@ -422,17 +415,17 @@ export class ReferencesManager {
// If there are only Confirmed results, complete the rename immediately.
let foundUnconfirmed: ReferenceInfo = referencesResult.referenceInfos.find(e => e.type !== ReferenceType.Confirmed);
if (!foundUnconfirmed) {
this.resultsCallback(referencesResult, true);
this.resultsCallback(referencesResult);
} else {
this.renameView.setData(referencesResult, this.groupByFile.Value, (result: ReferencesResult) => {
this.referencesCanceled = false;
this.resultsCallback(result, true);
this.resultsCallback(result);
});
this.renameView.show(true);
}
} else {
// Do nothing when rename is canceled while searching for references was in progress.
this.resultsCallback(null, true);
this.resultsCallback(null);
}
} else {
this.findAllRefsView.setData(referencesResult, referencesCanceled, this.groupByFile.Value);
Expand All @@ -448,12 +441,17 @@ export class ReferencesManager {
} else if (currentReferenceCommandMode === ReferencesCommandMode.Find) {
this.findAllRefsView.show(true);
}
if (referencesResult.isFinished && referencesRequestHasOccurred && !referencesCanceledWhilePreviewing) {
if (referencesResult.isFinished) {
this.lastResults = referencesResult;
this.referencesViewFindPending = true;
vscode.commands.executeCommand("references-view.refresh");
} else {
this.resultsCallback(referencesResult, referencesCanceledWhilePreviewing);
this.referencesFinished = true;
}
if (!this.referencesRefreshPending) {
if (referencesResult.isFinished && this.referencesRequestHasOccurred && !referencesRequestPending) {
this.referencesRefreshPending = true;
vscode.commands.executeCommand("references-view.refresh");
} else {
this.resultsCallback(referencesResult);
}
}
}
}
Expand Down

0 comments on commit e043c13

Please sign in to comment.