Skip to content

Commit e2fcfa1

Browse files
authored
fix(vscode-nes): improve cancellation chain handling (#713)
This change improves the cancellation chain handling in the NES trigger system by: 1. Adding proper cancellation token support to the provideNES method 2. Implementing cancellation token chaining between different components 3. Adding proper cleanup of cancellationTokenSource resources 4. Adding context validation to ensure results are only shown when context is still valid 5. Removing unnecessary debug logging The changes ensure that when a new request is triggered, previous ongoing requests are properly cancelled, and resources are cleaned up appropriately. Fixes the issue where stale results could be displayed after multiple rapid triggers.
1 parent 69c211c commit e2fcfa1

File tree

2 files changed

+73
-15
lines changed

2 files changed

+73
-15
lines changed

packages/vscode/src/nes/decoration-manager.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,6 @@ export class NESDecorationManager implements vscode.Disposable {
257257
const base64Image = Buffer.from(image).toString("base64");
258258
const dataUrl = `data:image/png;base64,${base64Image}`;
259259
logger.debug("Created image for decoration.");
260-
logger.trace("Image:", dataUrl);
261260

262261
// Check the longest line to determine the position of the image decoration.
263262
let longestLineChars = 0;

packages/vscode/src/nes/index.ts

Lines changed: 73 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,12 @@ export class NESProvider implements vscode.Disposable {
5656
async provideNES(
5757
document: vscode.TextDocument,
5858
selection: vscode.Selection,
59+
token?: vscode.CancellationToken | undefined,
5960
): Promise<NESSolution | undefined> {
61+
if (token?.isCancellationRequested) {
62+
return;
63+
}
64+
6065
const disabled =
6166
this.pochiConfiguration.advancedSettings.value.tabCompletion?.disabled;
6267
if (disabled) {
@@ -85,33 +90,32 @@ export class NESProvider implements vscode.Disposable {
8590
editHistory,
8691
});
8792

88-
if (this.fetching.value?.context.hash === context.hash) {
89-
logger.debug("Request is already ongoing with the same context");
90-
return;
91-
}
92-
9393
// Cancel the ongoing request if not matched
9494
if (this.fetching.value) {
9595
this.fetching.value.tokenSource.cancel();
9696
this.fetching.value.tokenSource.dispose();
9797
}
9898
const tokenSource = new vscode.CancellationTokenSource();
99-
const token = tokenSource.token;
100-
this.fetching.value = {
99+
if (token) {
100+
token.onCancellationRequested(() => tokenSource.cancel());
101+
}
102+
const cancellationToken = tokenSource.token;
103+
const fetching = {
101104
context,
102105
tokenSource,
103106
};
107+
this.fetching.value = fetching;
104108

105109
try {
106110
// Debounce
107111
const delay = 100; // ms
108112
await new Promise((resolve, reject) => {
109113
const timer = setTimeout(resolve, delay);
110-
if (token.isCancellationRequested) {
114+
if (cancellationToken.isCancellationRequested) {
111115
clearTimeout(timer);
112116
reject(new AbortError());
113117
} else {
114-
token.onCancellationRequested(() => {
118+
cancellationToken.onCancellationRequested(() => {
115119
clearTimeout(timer);
116120
reject(new AbortError());
117121
});
@@ -156,7 +160,7 @@ export class NESProvider implements vscode.Disposable {
156160
logger.debug("Failed to fetch completion", error);
157161
}
158162
} finally {
159-
if (this.fetching.value?.context.hash === context.hash) {
163+
if (this.fetching.value === fetching) {
160164
this.fetching.value.tokenSource.dispose();
161165
this.fetching.value = undefined;
162166
}
@@ -182,6 +186,7 @@ class NESInlineCompletionProvider
182186
private disposables: vscode.Disposable[] = [];
183187
private nesProvider: NESProvider | undefined;
184188
private nesDecorationManager: NESDecorationManager | undefined;
189+
private cancellationTokenSource: vscode.CancellationTokenSource | undefined;
185190

186191
initialize(
187192
nesProvider: NESProvider,
@@ -203,6 +208,15 @@ class NESInlineCompletionProvider
203208
context?: vscode.InlineCompletionContext | undefined,
204209
token?: vscode.CancellationToken | undefined,
205210
): Promise<vscode.InlineCompletionList | undefined> {
211+
logger.trace(
212+
`Function provideInlineCompletionItems called, document: ${document.uri.toString()}`,
213+
{ document, position, context, token },
214+
);
215+
if (this.cancellationTokenSource) {
216+
this.cancellationTokenSource.cancel();
217+
this.cancellationTokenSource.dispose();
218+
this.cancellationTokenSource = undefined;
219+
}
206220
this.nesDecorationManager?.dismiss();
207221

208222
if (token?.isCancellationRequested) {
@@ -217,14 +231,29 @@ class NESInlineCompletionProvider
217231
}
218232

219233
if (this.nesProvider) {
234+
const tokenSource = new vscode.CancellationTokenSource();
235+
if (token) {
236+
token.onCancellationRequested(() => tokenSource.cancel());
237+
}
238+
this.cancellationTokenSource = tokenSource;
239+
220240
logger.debug(
221241
`Trigger NES from InlineCompletionProvider, document: ${document.uri.toString()}`,
222242
);
223243
const version = document.version;
244+
const selection = new vscode.Selection(position, position);
224245
const solution = await this.nesProvider.provideNES(
225246
document,
226-
new vscode.Selection(position, position),
247+
selection,
248+
tokenSource.token,
227249
);
250+
251+
if (this.cancellationTokenSource !== tokenSource) {
252+
return undefined;
253+
}
254+
tokenSource.dispose();
255+
this.cancellationTokenSource = undefined;
256+
228257
if (solution && solution.items.length > 0) {
229258
// FIXME(zhiming): multi-choice not supported
230259
const solutionItem = solution.items[0];
@@ -241,10 +270,14 @@ class NESInlineCompletionProvider
241270
if (
242271
editor &&
243272
editor.document === document &&
244-
editor.document.version === version
273+
editor.document.version === version &&
274+
editor.selections[0].isEqual(selection)
275+
// FIXME(zhiming): use context hash to ensure context not changed
245276
) {
246277
logger.debug("Show result as decorations");
247278
this.nesDecorationManager.show(editor, solutionItem);
279+
} else {
280+
logger.debug("Will not show result as the context has changed.");
248281
}
249282
}
250283
}
@@ -265,6 +298,7 @@ class NESEditorListener implements vscode.Disposable {
265298
private disposables: vscode.Disposable[] = [];
266299
private nesProvider: NESProvider | undefined;
267300
private nesDecorationManager: NESDecorationManager | undefined;
301+
private cancellationTokenSource: vscode.CancellationTokenSource | undefined;
268302

269303
initialize(
270304
nesProvider: NESProvider,
@@ -281,7 +315,14 @@ class NESEditorListener implements vscode.Disposable {
281315
if (!vscode.languages.match(DocumentSelector, document)) {
282316
return;
283317
}
318+
319+
if (this.cancellationTokenSource) {
320+
this.cancellationTokenSource.cancel();
321+
this.cancellationTokenSource.dispose();
322+
this.cancellationTokenSource = undefined;
323+
}
284324
this.nesDecorationManager?.dismiss();
325+
285326
if (
286327
(event.kind === vscode.TextEditorSelectionChangeKind.Mouse ||
287328
event.kind === vscode.TextEditorSelectionChangeKind.Keyboard) &&
@@ -290,14 +331,26 @@ class NESEditorListener implements vscode.Disposable {
290331
) {
291332
// Trigger when user selects a range with mouse or keyboard
292333
if (this.nesProvider) {
334+
const tokenSource = new vscode.CancellationTokenSource();
335+
this.cancellationTokenSource = tokenSource;
336+
293337
logger.debug(
294338
`Trigger NES from TextEditorSelectionChange, document: ${document.uri.toString()}`,
295339
);
296340
const version = document.version;
341+
const selection = event.selections[0];
297342
const solution = await this.nesProvider.provideNES(
298343
document,
299-
event.selections[0],
344+
selection,
345+
tokenSource.token,
300346
);
347+
348+
if (this.cancellationTokenSource !== tokenSource) {
349+
return undefined;
350+
}
351+
tokenSource.dispose();
352+
this.cancellationTokenSource = undefined;
353+
301354
if (
302355
solution &&
303356
solution.items.length > 0 &&
@@ -307,12 +360,18 @@ class NESEditorListener implements vscode.Disposable {
307360
if (
308361
editor &&
309362
editor.document === document &&
310-
editor.document.version === version
363+
editor.document.version === version &&
364+
editor.selections[0].isEqual(selection)
365+
// FIXME(zhiming): use context hash to ensure context not changed
311366
) {
312367
logger.debug("Show result as decorations");
313368
// FIXME(zhiming): multi-choice not supported
314369
const solutionItem = solution.items[0];
315370
this.nesDecorationManager.show(event.textEditor, solutionItem);
371+
} else {
372+
logger.debug(
373+
"Will not show result as the context has changed.",
374+
);
316375
}
317376
}
318377
}

0 commit comments

Comments
 (0)