Fix version-directory cleanup race and run cloudformation-validate in a worker thread

package.json

@@ -5,20 +5,8 @@
     "id": "aws-cloudformation-languageserver",
     "description": "AWS CloudFormation Language Server",
     "publisher": "aws",
+    "author": "Amazon Web Services (AWS)",
     "license": "Apache-2.0",
-    "private": true,
-    "author": {
-        "name": "Amazon Web Services",
-        "url": "https://aws.amazon.com"
-    },
-    "repository": {
-        "type": "git",
-        "url": "https://github.com/aws-cloudformation/cloudformation-languageserver"
-    },
-    "homepage": "https://github.com/aws-cloudformation/cloudformation-languageserver#readme",
-    "bugs": {
-        "url": "https://github.com/aws-cloudformation/cloudformation-languageserver/issues"
-    },
     "main": "src/app/standalone.ts",
     "engines": {
         "node": ">=24.19.0 <25",
@@ -72,10 +60,13 @@
         "@aws/cloudformation-validate": "1.12.0",
         "@base2/pretty-print-object": "1.0.2",
         "@opentelemetry/api": "1.9.1",
-        "@opentelemetry/exporter-metrics-otlp-http": "0.221.0",
-        "@opentelemetry/instrumentation-runtime-node": "0.34.0",
+        "@opentelemetry/auto-instrumentations-node": "0.79.0",
         "@opentelemetry/resources": "2.10.0",
+        "@opentelemetry/sdk-logs": "0.221.0",
         "@opentelemetry/sdk-metrics": "2.10.0",
+        "@opentelemetry/sdk-node": "0.221.0",
+        "@opentelemetry/sdk-trace-base": "2.10.0",
+        "@opentelemetry/sdk-trace-node": "2.10.0",
         "@smithy/util-waiter": "4.6.2",
         "@tree-sitter-grammars/tree-sitter-yaml": "0.7.1",
         "archiver": "7.0.1",
@@ -157,10 +148,14 @@
     "externalDependencies": [
         "@aws/cloudformation-validate",
         "@opentelemetry/api",
-        "@opentelemetry/exporter-metrics-otlp-http",
-        "@opentelemetry/instrumentation-runtime-node",
+        "@opentelemetry/auto-instrumentations-node",
+        "@opentelemetry/instrumentation-pino",
         "@opentelemetry/resources",
+        "@opentelemetry/sdk-logs",
         "@opentelemetry/sdk-metrics",
+        "@opentelemetry/sdk-node",
+        "@opentelemetry/sdk-trace-base",
+        "@opentelemetry/sdk-trace-node",
         "@tree-sitter-grammars/tree-sitter-yaml",
         "lmdb",
         "pino",

src/datastore/FileStoreFactory.ts

@@ -10,7 +10,7 @@ import { DataStore, DataStoreFactory, PersistedStores, StoreName, TotalMaxDatast
 import { encryptionKey } from './file/Encryption';
 import { KeyedFileStore } from './file/KeyedFileStore';
 import { recordDiscardedData, recordDiskUsage, recordOutOfDiskFailure, StoreOperation } from './Utils';
-import { isOlderVersionDirectory } from './VersionDirectory';
+import { isOlderVersionDirectory, isRecentlyUsed, touchHeartbeat } from './VersionDirectory';
 
 export class FileStoreFactory implements DataStoreFactory {
     private readonly log = LoggerFactory.getLogger('FileStore.Global');
@@ -34,6 +34,7 @@ export class FileStoreFactory implements DataStoreFactory {
         if (!existsSync(this.fileDbDir)) {
             mkdirSync(this.fileDbDir, { recursive: true });
         }
+        this.recordHeartbeat();
 
         for (const store of storeNames) {
             this.stores.set(
@@ -88,6 +89,7 @@ export class FileStoreFactory implements DataStoreFactory {
     private emitMetrics(): void {
         if (this.closed) return;
 
+        this.recordHeartbeat();
         this.telemetry.histogram('version', VersionNumber);
         this.telemetry.histogram('env.entries', this.stores.size);
 
@@ -114,16 +116,30 @@ export class FileStoreFactory implements DataStoreFactory {
         }
     }
 
+    private recordHeartbeat(): void {
+        if (!touchHeartbeat(this.fileDbDir)) {
+            this.telemetry.count('heartbeat.error', 1);
+        }
+    }
+
     private cleanupOldVersions(): void {
         if (this.closed || !existsSync(this.fileDbRoot)) return;
 
         const entries = readdirSync(this.fileDbRoot, { withFileTypes: true });
         for (const entry of entries) {
             try {
-                if (entry.isDirectory() && isOlderVersionDirectory(entry.name, VersionNumber)) {
-                    this.telemetry.count('oldVersion.cleanup.count', 1);
-                    rmSync(join(this.fileDbRoot, entry.name), { recursive: true, force: true });
+                if (!entry.isDirectory() || !isOlderVersionDirectory(entry.name, VersionNumber)) {
+                    continue;
                 }
+
+                const versionDir = join(this.fileDbRoot, entry.name);
+                if (isRecentlyUsed(versionDir)) {
+                    this.telemetry.count('oldVersion.cleanup.skipped', 1);
+                    continue;
+                }
+
+                this.telemetry.count('oldVersion.cleanup.count', 1);
+                rmSync(versionDir, { recursive: true, force: true });
             } catch (error) {
                 this.log.error(error, 'Failed to cleanup old FileDB versions');
                 this.telemetry.count('oldVersion.cleanup.error', 1);

src/datastore/LMDBStoreFactory.ts

@@ -16,7 +16,7 @@ import { LMDBOwnershipTracker } from './lmdb/OwnershipTracker';
 import { stats } from './lmdb/Stats';
 import { encryptionStrategy } from './lmdb/Utils';
 import { recordDiscardedData, recordDiskUsage, recordOutOfDiskFailure, StoreOperation } from './Utils';
-import { isOlderVersionDirectory } from './VersionDirectory';
+import { isOlderVersionDirectory, isRecentlyUsed, touchHeartbeat } from './VersionDirectory';
 
 const MetricsIntervalMs = 60 * 1000;
 const CleanupDelayMs = 2 * 60 * 1000;
@@ -91,9 +91,16 @@ export class LMDBStoreFactory implements DataStoreFactory {
         // The env and stores opened successfully — promote the marker out of `opening`
         // so an abrupt kill from now on is not mistaken for a startup crash.
         this.ownershipTracker.markRunning();
+        this.recordHeartbeat();
         this.scheduleBackgroundTasks();
     }
 
+    private recordHeartbeat(): void {
+        if (!touchHeartbeat(this.lmdbVersionDir)) {
+            this.telemetry.count('heartbeat.error', 1);
+        }
+    }
+
     get(store: StoreName): DataStore {
         const val = this.stores.get(store);
         if (val === undefined) {
@@ -346,13 +353,21 @@ export class LMDBStoreFactory implements DataStoreFactory {
         for (const entry of entries) {
             try {
                 if (
-                    entry.isDirectory() &&
-                    isOlderVersionDirectory(entry.name, VersionNumber) &&
-                    entry.name !== LMDBOwnershipTracker.DirName
+                    !entry.isDirectory() ||
+                    !isOlderVersionDirectory(entry.name, VersionNumber) ||
+                    entry.name === LMDBOwnershipTracker.DirName
                 ) {
-                    this.telemetry.count('oldVersion.cleanup.count', 1);
-                    rmSync(join(this.lmdbDir, entry.name), { recursive: true, force: true });
+                    continue;
                 }
+
+                const versionDir = join(this.lmdbDir, entry.name);
+                if (isRecentlyUsed(versionDir)) {
+                    this.telemetry.count('oldVersion.cleanup.skipped', 1);
+                    continue;
+                }
+
+                this.telemetry.count('oldVersion.cleanup.count', 1);
+                rmSync(versionDir, { recursive: true, force: true });
             } catch (error) {
                 this.log.error(error, 'Failed to cleanup old LMDB versions');
                 this.telemetry.count('oldVersion.cleanup.error', 1);
@@ -365,6 +380,8 @@ export class LMDBStoreFactory implements DataStoreFactory {
             return;
         }
 
+        this.recordHeartbeat();
+
         try {
             const staleLocks = this.env.readerCheck();
             if (staleLocks > 0) {

src/datastore/VersionDirectory.ts

@@ -1,5 +1,11 @@
+import { closeSync, openSync, statSync, utimesSync } from 'fs';
+import { join } from 'path';
+
 const VersionDirectoryPattern = /^v(\d+)$/;
 
+export const HeartbeatFileName = '.heartbeat';
+export const RecentUseWindowMs = 24 * 60 * 60 * 1000;
+
 /**
  * True only for a strict `v<number>` directory whose version is below {@link currentVersion}.
  *
@@ -22,3 +28,43 @@ export function isOlderVersionDirectory(directoryName: string, currentVersion: n
 
     return version < currentVersion;
 }
+
+/**
+ * Marks {@link versionDir} as in use by a live process. A newer binary on the same machine checks this
+ * before deleting an older version directory, so an older process that is still running (another IDE,
+ * a window that has not restarted yet) does not lose its store underneath it.
+ *
+ * @returns false when the marker could not be written (for example the directory is gone)
+ */
+export function touchHeartbeat(versionDir: string, now = new Date()): boolean {
+    const heartbeat = join(versionDir, HeartbeatFileName);
+    try {
+        try {
+            utimesSync(heartbeat, now, now);
+        } catch {
+            closeSync(openSync(heartbeat, 'a'));
+            utimesSync(heartbeat, now, now);
+        }
+        return true;
+    } catch {
+        return false;
+    }
+}
+
+/**
+ * True when the directory was used within {@link windowMs}. The heartbeat written by
+ * {@link touchHeartbeat} is authoritative; the directory's own mtime is the fallback for stores written
+ * by binaries that predate the heartbeat. Unreadable directories are treated as in use.
+ */
+export function isRecentlyUsed(versionDir: string, windowMs = RecentUseWindowMs, now = Date.now()): boolean {
+    const lastUsed = Math.max(mtimeMs(join(versionDir, HeartbeatFileName)), mtimeMs(versionDir));
+    return lastUsed === Number.POSITIVE_INFINITY || now - lastUsed < windowMs;
+}
+
+function mtimeMs(path: string): number {
+    try {
+        return statSync(path).mtimeMs;
+    } catch (error) {
+        return (error as NodeJS.ErrnoException).code === 'ENOENT' ? Number.NEGATIVE_INFINITY : Number.POSITIVE_INFINITY;
+    }
+}

src/services/cfnLint/PyodideWorkerManager.ts

@@ -8,6 +8,8 @@ import { ScopedTelemetry } from '../../telemetry/ScopedTelemetry';
 import { Telemetry } from '../../telemetry/TelemetryDecorator';
 import {
     CfnLintInitializationError,
+    WorkerExitError,
+    WorkerFailureError,
     WorkerNotInitializedError,
     WorkerShutdownError,
 } from '../../utils/errors/ErrorClasses';
@@ -119,7 +121,7 @@ export class PyodideWorkerManager {
 
                         // Reject any pending tasks
                         for (const task of this.tasks.values()) {
-                            task.reject(new Error(`Worker exited unexpectedly with code ${code}`));
+                            task.reject(new WorkerExitError(code));
                         }
                         this.tasks.clear();
                     }
@@ -131,7 +133,7 @@ export class PyodideWorkerManager {
                 // Set up error handler
                 this.worker.on('error', (error: Error) => {
                     this.log.error(error, 'Worker error');
-                    reject(new Error(`Worker error: ${error.message}`));
+                    reject(new WorkerFailureError(error));
                 });
 
                 // Initialize Pyodide in the worker