Skip to content

Commit 0992a2b

Browse files
pmourapmoura
committed
Fix race condition where code navigation and refactoring operations could hang with workspaces with a large number of directories due to extension startup workspace cleanup process
1 parent b6e67f8 commit 0992a2b

9 files changed

Lines changed: 254 additions & 202 deletions

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Changelog
22

3+
## [0.88.4]
4+
5+
* Fix race condition where code navigation and refactoring operations could hang with workspaces with a large number of directories due to extension startup workspace cleanup process
6+
37
## [0.88.3]
48

59
* Fix code navigation features to be more robust to concurrent requests

‎package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"name": "logtalk-for-vscode",
33
"displayName": "Logtalk for VSCode",
44
"description": "Logtalk programming support",
5-
"version": "0.88.3",
5+
"version": "0.88.4",
66
"publisher": "LogtalkDotOrg",
77
"icon": "images/logtalk.png",
88
"license": "MIT",
@@ -1332,7 +1332,7 @@
13321332
"compile": "tsc -watch -p ./",
13331333
"test": "tsc ./tests/runTest.ts",
13341334
"vsix:make": "vsce package --baseImagesUrl https://raw.githubusercontent.com/llvm/llvm-project/master/clang-tools-extra/clangd/clients/clangd-vscode/",
1335-
"vsix:install": "code --install-extension logtalk-for-vscode-0.88.3.vsix"
1335+
"vsix:install": "code --install-extension logtalk-for-vscode-0.88.4.vsix"
13361336
},
13371337
"devDependencies": {
13381338
"@types/bluebird": "^3.5.38",

‎src/features/callHierarchyProvider.ts‎

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,32 +22,40 @@ import * as path from "path";
2222

2323
export class LogtalkCallHierarchyProvider implements CallHierarchyProvider {
2424
private logger = getLogger();
25-
private disposables: Disposable[] = [];
25+
private static readonly TEMP_FILES = [
26+
".vscode_callers",
27+
".vscode_callers_done",
28+
".vscode_callees",
29+
".vscode_callees_done"
30+
];
31+
private static startupCleanupPromise: Promise<void> | null = null;
32+
private static workspaceFoldersListener: Disposable | null = null;
2633

2734
constructor() {
28-
// Delete any temporary files from previous sessions in all workspace folders
29-
const files = [
30-
".vscode_callers",
31-
".vscode_callers_done",
32-
".vscode_callees",
33-
".vscode_callees_done"
34-
];
35-
// Fire-and-forget cleanup - errors are logged internally
36-
if (workspace.workspaceFolders) {
37-
for (const wf of workspace.workspaceFolders) {
38-
Utils.cleanupTemporaryFiles(wf.uri.fsPath, files);
39-
}
35+
// Ensure startup marker cleanup runs once and can be awaited before lookups.
36+
if (!LogtalkCallHierarchyProvider.startupCleanupPromise) {
37+
LogtalkCallHierarchyProvider.startupCleanupPromise = this.cleanupStartupTemporaryFiles();
4038
}
4139

42-
// Clean up any temporary files when folders are added to the workspace
43-
const workspaceFoldersListener = workspace.onDidChangeWorkspaceFolders((event) => {
44-
// For each added workspace folder, run the cleanup using the folder path
45-
// Fire-and-forget cleanup - errors are logged internally
46-
for (const wf of event.added) {
47-
Utils.cleanupTemporaryFiles(wf.uri.fsPath, files);
48-
}
49-
});
50-
this.disposables.push(workspaceFoldersListener);
40+
// Register the workspace-folder listener only once to avoid duplicate cleanup runs.
41+
if (!LogtalkCallHierarchyProvider.workspaceFoldersListener) {
42+
LogtalkCallHierarchyProvider.workspaceFoldersListener = workspace.onDidChangeWorkspaceFolders((event) => {
43+
for (const wf of event.added) {
44+
// Fire-and-forget for newly added folders; startup is the critical synchronized path.
45+
Utils.cleanupTemporaryFiles(wf.uri.fsPath, LogtalkCallHierarchyProvider.TEMP_FILES);
46+
}
47+
});
48+
}
49+
}
50+
51+
private async cleanupStartupTemporaryFiles(): Promise<void> {
52+
if (!workspace.workspaceFolders) {
53+
return;
54+
}
55+
56+
for (const wf of workspace.workspaceFolders) {
57+
await Utils.cleanupTemporaryFiles(wf.uri.fsPath, LogtalkCallHierarchyProvider.TEMP_FILES);
58+
}
5159
}
5260

5361
/**
@@ -154,6 +162,7 @@ export class LogtalkCallHierarchyProvider implements CallHierarchyProvider {
154162
position: Position,
155163
token: CancellationToken
156164
): ProviderResult<CallHierarchyItem> {
165+
// prepareCallHierarchy is synchronous by API contract, so we cannot await here.
157166
let predicate = Utils.getCallUnderCursor(doc, position);
158167
if (!predicate) {
159168
return null;
@@ -194,6 +203,10 @@ export class LogtalkCallHierarchyProvider implements CallHierarchyProvider {
194203
item: CallHierarchyItem,
195204
token: CancellationToken
196205
): Promise<CallHierarchyIncomingCall[]> {
206+
if (LogtalkCallHierarchyProvider.startupCleanupPromise) {
207+
await LogtalkCallHierarchyProvider.startupCleanupPromise;
208+
}
209+
197210
let callers: CallHierarchyIncomingCall[] = [];
198211
let file = item.uri.fsPath;
199212
let predicate = item.name;
@@ -250,6 +263,10 @@ export class LogtalkCallHierarchyProvider implements CallHierarchyProvider {
250263
item: CallHierarchyItem,
251264
token: CancellationToken
252265
): Promise<CallHierarchyOutgoingCall[]> {
266+
if (LogtalkCallHierarchyProvider.startupCleanupPromise) {
267+
await LogtalkCallHierarchyProvider.startupCleanupPromise;
268+
}
269+
253270
let callees: CallHierarchyOutgoingCall[] = [];
254271
let file = item.uri.fsPath;
255272
let predicate = item.name;
@@ -303,13 +320,6 @@ export class LogtalkCallHierarchyProvider implements CallHierarchyProvider {
303320
}
304321

305322
public dispose(): void {
306-
for (const d of this.disposables) {
307-
try {
308-
d.dispose();
309-
} catch (err) {
310-
this.logger.error('Error disposing resource:', err);
311-
}
312-
}
313-
this.disposables = [];
323+
// Shared listener is intentionally process-scoped and initialized once.
314324
}
315325
}

‎src/features/declarationProvider.ts‎

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,37 +17,49 @@ import * as path from "path";
1717

1818
export class LogtalkDeclarationProvider implements DeclarationProvider {
1919
private logger = getLogger();
20-
private disposables: Disposable[] = [];
20+
private static readonly TEMP_FILES = [
21+
".vscode_declaration",
22+
".vscode_declaration_done"
23+
];
24+
private static startupCleanupPromise: Promise<void> | null = null;
25+
private static workspaceFoldersListener: Disposable | null = null;
2126

2227
constructor() {
23-
// Delete any temporary files from previous sessions in all workspace folders
24-
const files = [
25-
".vscode_declaration",
26-
".vscode_declaration_done"
27-
];
28-
// Fire-and-forget cleanup - errors are logged internally
29-
if (workspace.workspaceFolders) {
30-
for (const wf of workspace.workspaceFolders) {
31-
Utils.cleanupTemporaryFiles(wf.uri.fsPath, files);
32-
}
28+
// Ensure startup marker cleanup runs once and can be awaited before lookups.
29+
if (!LogtalkDeclarationProvider.startupCleanupPromise) {
30+
LogtalkDeclarationProvider.startupCleanupPromise = this.cleanupStartupTemporaryFiles();
3331
}
3432

35-
// Clean up any temporary files when folders are added to the workspace
36-
const workspaceFoldersListener = workspace.onDidChangeWorkspaceFolders((event) => {
37-
// For each added workspace folder, run the cleanup using the folder path
38-
// Fire-and-forget cleanup - errors are logged internally
39-
for (const wf of event.added) {
40-
Utils.cleanupTemporaryFiles(wf.uri.fsPath, files);
41-
}
42-
});
43-
this.disposables.push(workspaceFoldersListener);
33+
// Register the workspace-folder listener only once to avoid duplicate cleanup runs.
34+
if (!LogtalkDeclarationProvider.workspaceFoldersListener) {
35+
LogtalkDeclarationProvider.workspaceFoldersListener = workspace.onDidChangeWorkspaceFolders((event) => {
36+
for (const wf of event.added) {
37+
// Fire-and-forget for newly added folders; startup is the critical synchronized path.
38+
Utils.cleanupTemporaryFiles(wf.uri.fsPath, LogtalkDeclarationProvider.TEMP_FILES);
39+
}
40+
});
41+
}
42+
}
43+
44+
private async cleanupStartupTemporaryFiles(): Promise<void> {
45+
if (!workspace.workspaceFolders) {
46+
return;
47+
}
48+
49+
for (const wf of workspace.workspaceFolders) {
50+
await Utils.cleanupTemporaryFiles(wf.uri.fsPath, LogtalkDeclarationProvider.TEMP_FILES);
51+
}
4452
}
4553

4654
public async provideDeclaration(
4755
doc: TextDocument,
4856
position: Position,
4957
token: CancellationToken
5058
): Promise<Location | null> {
59+
if (LogtalkDeclarationProvider.startupCleanupPromise) {
60+
await LogtalkDeclarationProvider.startupCleanupPromise;
61+
}
62+
5163
const lineText = doc.lineAt(position.line).text.trim();
5264
if (lineText.startsWith("%")) {
5365
return null;
@@ -90,13 +102,6 @@ export class LogtalkDeclarationProvider implements DeclarationProvider {
90102
}
91103

92104
public dispose(): void {
93-
for (const d of this.disposables) {
94-
try {
95-
d.dispose();
96-
} catch (err) {
97-
this.logger.error('Error disposing resource:', err);
98-
}
99-
}
100-
this.disposables = [];
105+
// Shared listener is intentionally process-scoped and initialized once.
101106
}
102107
}

‎src/features/definitionProvider.ts‎

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -19,33 +19,41 @@ import * as fs from "fs";
1919

2020
export class LogtalkDefinitionProvider implements DefinitionProvider {
2121
private logger = getLogger();
22-
private disposables: Disposable[] = [];
22+
private static readonly TEMP_FILES = [
23+
".vscode_definition",
24+
".vscode_definition_done"
25+
];
26+
private static startupCleanupPromise: Promise<void> | null = null;
27+
private static workspaceFoldersListener: Disposable | null = null;
2328

2429
// Extensions to try when resolving file paths (in order of priority)
2530
private static readonly FILE_EXTENSIONS = ['.lgt', '.logtalk', '.pl', '.prolog', ''];
2631

2732
constructor() {
28-
// Delete any temporary files from previous sessions in all workspace folders
29-
const files = [
30-
".vscode_definition",
31-
".vscode_definition_done"
32-
];
33-
// Fire-and-forget cleanup - errors are logged internally
34-
if (workspace.workspaceFolders) {
35-
for (const wf of workspace.workspaceFolders) {
36-
Utils.cleanupTemporaryFiles(wf.uri.fsPath, files);
37-
}
33+
// Ensure startup marker cleanup runs once and can be awaited before lookups.
34+
if (!LogtalkDefinitionProvider.startupCleanupPromise) {
35+
LogtalkDefinitionProvider.startupCleanupPromise = this.cleanupStartupTemporaryFiles();
3836
}
3937

40-
// Clean up any temporary files when folders are added to the workspace
41-
const workspaceFoldersListener = workspace.onDidChangeWorkspaceFolders((event) => {
42-
// For each added workspace folder, run the cleanup using the folder path
43-
// Fire-and-forget cleanup - errors are logged internally
44-
for (const wf of event.added) {
45-
Utils.cleanupTemporaryFiles(wf.uri.fsPath, files);
46-
}
47-
});
48-
this.disposables.push(workspaceFoldersListener);
38+
// Register the workspace-folder listener only once to avoid duplicate cleanup runs.
39+
if (!LogtalkDefinitionProvider.workspaceFoldersListener) {
40+
LogtalkDefinitionProvider.workspaceFoldersListener = workspace.onDidChangeWorkspaceFolders((event) => {
41+
for (const wf of event.added) {
42+
// Fire-and-forget for newly added folders; startup is the critical synchronized path.
43+
Utils.cleanupTemporaryFiles(wf.uri.fsPath, LogtalkDefinitionProvider.TEMP_FILES);
44+
}
45+
});
46+
}
47+
}
48+
49+
private async cleanupStartupTemporaryFiles(): Promise<void> {
50+
if (!workspace.workspaceFolders) {
51+
return;
52+
}
53+
54+
for (const wf of workspace.workspaceFolders) {
55+
await Utils.cleanupTemporaryFiles(wf.uri.fsPath, LogtalkDefinitionProvider.TEMP_FILES);
56+
}
4957
}
5058

5159
/**
@@ -211,6 +219,10 @@ export class LogtalkDefinitionProvider implements DefinitionProvider {
211219
position: Position,
212220
token: CancellationToken
213221
): Promise<Location | null> {
222+
if (LogtalkDefinitionProvider.startupCleanupPromise) {
223+
await LogtalkDefinitionProvider.startupCleanupPromise;
224+
}
225+
214226
if (window.activeTextEditor?.document === doc && window.activeTextEditor.selection.active.line !== position.line) {
215227
return null;
216228
}
@@ -273,13 +285,6 @@ export class LogtalkDefinitionProvider implements DefinitionProvider {
273285
}
274286

275287
public dispose(): void {
276-
for (const d of this.disposables) {
277-
try {
278-
d.dispose();
279-
} catch (err) {
280-
this.logger.error('Error disposing resource:', err);
281-
}
282-
}
283-
this.disposables = [];
288+
// Shared listener is intentionally process-scoped and initialized once.
284289
}
285290
}

‎src/features/implementationProvider.ts‎

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -19,37 +19,49 @@ import * as path from "path";
1919

2020
export class LogtalkImplementationProvider implements ImplementationProvider {
2121
private logger = getLogger();
22-
private disposables: Disposable[] = [];
22+
private static readonly TEMP_FILES = [
23+
".vscode_implementations",
24+
".vscode_implementations_done"
25+
];
26+
private static startupCleanupPromise: Promise<void> | null = null;
27+
private static workspaceFoldersListener: Disposable | null = null;
2328

2429
constructor() {
25-
// Delete any temporary files from previous sessions in all workspace folders
26-
const files = [
27-
".vscode_implementations",
28-
".vscode_implementations_done"
29-
];
30-
// Fire-and-forget cleanup - errors are logged internally
31-
if (workspace.workspaceFolders) {
32-
for (const wf of workspace.workspaceFolders) {
33-
Utils.cleanupTemporaryFiles(wf.uri.fsPath, files);
34-
}
30+
// Ensure startup marker cleanup runs once and can be awaited before lookups.
31+
if (!LogtalkImplementationProvider.startupCleanupPromise) {
32+
LogtalkImplementationProvider.startupCleanupPromise = this.cleanupStartupTemporaryFiles();
3533
}
3634

37-
// Clean up any temporary files when folders are added to the workspace
38-
const workspaceFoldersListener = workspace.onDidChangeWorkspaceFolders((event) => {
39-
// For each added workspace folder, run the cleanup using the folder path
40-
// Fire-and-forget cleanup - errors are logged internally
41-
for (const wf of event.added) {
42-
Utils.cleanupTemporaryFiles(wf.uri.fsPath, files);
43-
}
44-
});
45-
this.disposables.push(workspaceFoldersListener);
35+
// Register the workspace-folder listener only once to avoid duplicate cleanup runs.
36+
if (!LogtalkImplementationProvider.workspaceFoldersListener) {
37+
LogtalkImplementationProvider.workspaceFoldersListener = workspace.onDidChangeWorkspaceFolders((event) => {
38+
for (const wf of event.added) {
39+
// Fire-and-forget for newly added folders; startup is the critical synchronized path.
40+
Utils.cleanupTemporaryFiles(wf.uri.fsPath, LogtalkImplementationProvider.TEMP_FILES);
41+
}
42+
});
43+
}
44+
}
45+
46+
private async cleanupStartupTemporaryFiles(): Promise<void> {
47+
if (!workspace.workspaceFolders) {
48+
return;
49+
}
50+
51+
for (const wf of workspace.workspaceFolders) {
52+
await Utils.cleanupTemporaryFiles(wf.uri.fsPath, LogtalkImplementationProvider.TEMP_FILES);
53+
}
4654
}
4755

4856
public async provideImplementation(
4957
doc: TextDocument,
5058
position: Position,
5159
token: CancellationToken
5260
): Promise<Definition | LocationLink[]> {
61+
if (LogtalkImplementationProvider.startupCleanupPromise) {
62+
await LogtalkImplementationProvider.startupCleanupPromise;
63+
}
64+
5365
const lineText = doc.lineAt(position.line).text.trim();
5466
if (lineText.startsWith("%")) {
5567
return null;
@@ -96,13 +108,6 @@ export class LogtalkImplementationProvider implements ImplementationProvider {
96108
}
97109

98110
public dispose(): void {
99-
for (const d of this.disposables) {
100-
try {
101-
d.dispose();
102-
} catch (err) {
103-
this.logger.error('Error disposing resource:', err);
104-
}
105-
}
106-
this.disposables = [];
111+
// Shared listener is intentionally process-scoped and initialized once.
107112
}
108113
}

0 commit comments

Comments
 (0)