fix: race condition in workspace scanner (#212)

This time without the infinite loop for workspaces with circular references.
wkillerud · Aug 24, 2024 · 099a54b · 099a54b
1 parent e0b0af6
commit 099a54b
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 20 deletions.
diff --git a/packages/language-server/src/workspace-scanner.ts b/packages/language-server/src/workspace-scanner.ts
@@ -53,24 +53,22 @@ export default class WorkspaceScanner {
 			uri = URI.parse(file.toString().replace("/static/extensions/fs", ""));
 		}
 
-		const alreadyParsed = this.#ls.hasCached(uri);
-		if (alreadyParsed) {
-			// The same file may be referenced by multiple other files,
-			// so skip doing the parsing work if it's already been done.
-			// Changes to the file are handled by the `update` method.
-			return;
-		}
-
 		try {
-			// TODO: figure out what caused the loop
-			const content = await this.#fs.readFile(uri);
+			let document: TextDocument | null | undefined =
+				this.#ls.getCachedTextDocument(uri);
 
-			const document = getSCSSRegionsDocument(
-				TextDocument.create(uri.toString(), "scss", 1, content),
-			);
-			if (!document) return;
+			if (!document) {
+				const content = await this.#fs.readFile(uri);
+
+				document = getSCSSRegionsDocument(
+					TextDocument.create(uri.toString(), "scss", 1, content),
+				);
+				if (!document) return;
+			}
 
+			this.#ls.parseStylesheet(document);
 			const links = await this.#ls.findDocumentLinks(document);
+
 			for (const link of links) {
 				if (
 					!link.target ||
@@ -81,8 +79,17 @@ export default class WorkspaceScanner {
 					continue;
 				}
 
+				let uri = URI.parse(link.target);
+				let visited: TextDocument | null | undefined =
+					this.#ls.getCachedTextDocument(uri);
+
+				if (visited) {
+					// avoid infinite loop if circular references
+					continue;
+				}
+
 				try {
-					await this.parse(URI.parse(link.target), depth + 1);
+					await this.parse(uri, depth + 1);
 				} catch {
 					// do nothing
 				}

diff --git a/packages/language-services/src/language-services-types.ts b/packages/language-services/src/language-services-types.ts
@@ -133,7 +133,7 @@ export interface LanguageService {
 		range: Range,
 		context?: CodeActionContext,
 	): Promise<CodeAction[]>;
-	hasCached(uri: URI): boolean;
+	getCachedTextDocument(uri: URI): TextDocument | undefined;
 	/**
 	 * Utility function to reparse an updated document.
 	 * Like {@link LanguageService.parseStylesheet}, but returns nothing.

diff --git a/packages/language-services/src/language-services.ts b/packages/language-services/src/language-services.ts
@@ -160,8 +160,8 @@ class LanguageServiceImpl implements LanguageService {
 		return this.#findSymbols.findWorkspaceSymbols(query);
 	}
 
-	hasCached(uri: URI): boolean {
-		return this.#cache.has(uri.toString());
+	getCachedTextDocument(uri: URI): TextDocument | undefined {
+		return this.#cache.getDocument(uri.toString());
 	}
 
 	getColorPresentations(document: TextDocument, color: Color, range: Range) {

diff --git a/vscode-extension/package.json b/vscode-extension/package.json
@@ -131,7 +131,7 @@
     "build": "webpack --mode development",
     "start:web": "vscode-test-web --browserType=chromium --extensionDevelopmentPath=.",
     "lint": "eslint \"**/*.ts\" --cache",
-    "test:e2e": "node ./test/e2e/runTest.js",
+    "test:e2e": "node ./test/e2e/runTest.js && node ./test/new-e2e/runTest.js",
     "pretest:web": "webpack --config ./webpack.test-web.config.js ",
     "test:web": "node ./test/web/runTest.js"
   },
@@ -141,4 +141,4 @@
     "publisherId": "02638283-c13a-4acf-9f26-24bdcfdfce24",
     "isPreReleaseVersion": false
   }
-}
+}