fix(sdk): simulator lockfile race condition during release (#6935)

In some cases, the lockfile would try to update the lockfile utimes while it was being released due to the async nature of the implementation. This changeset refactors the implementation so it's purely sync.

libs/wingsdk/src/simulator/lockfile.ts

@@ -1,5 +1,5 @@
-import fs, { type FileHandle } from "fs/promises";
-import path from "path";
+import fs from "node:fs";
+import path from "node:path";
 import { isNodeError } from "./util.js";
 
 /**
@@ -29,9 +29,10 @@ export interface LockfileProps {
 
 export class Lockfile {
   private path: string;
-  private lockfile: FileHandle | undefined;
+  private lockfile: number | undefined;
   private timeout: NodeJS.Timeout | undefined;
   private lastMtime: number | undefined;
+  private compromised = false;
   private onCompromised?: (
     reason: string,
     error?: unknown
@@ -42,18 +43,20 @@ export class Lockfile {
     this.onCompromised = props.onCompromised;
   }
 
-  public async lock() {
+  public lock() {
     if (this.lockfile) {
-      return; // already locked
+      return;
     }
 
-    await fs.mkdir(path.dirname(this.path), { recursive: true });
+    this.compromised = false;
+
+    fs.mkdirSync(path.dirname(this.path), { recursive: true });
 
     try {
       // If lockfile exists but it's stale, remove it.
-      const stats = await fs.stat(this.path);
+      const stats = fs.statSync(this.path);
       if (Date.now() > stats.mtimeMs + STALE_THRESHOLD) {
-        await fs.rm(this.path);
+        fs.rmSync(this.path, { force: true });
       }
     } catch (error) {
       if (isNodeError(error) && error.code === "ENOENT") {
@@ -67,7 +70,7 @@ export class Lockfile {
     }
 
     try {
-      this.lockfile = await fs.open(this.path, "wx");
+      this.lockfile = fs.openSync(this.path, "wx");
     } catch (error) {
       if (isNodeError(error) && error.code === "EEXIST") {
         throw new Error(
@@ -83,73 +86,73 @@ export class Lockfile {
 
   private updateLockfileLater() {
     this.timeout = setTimeout(() => {
-      void (async () => {
-        // If we already updated our lockfile once, we need to check if it got compromised.
-        if (this.lastMtime) {
-          // Check if the lockfile got compromised because we were too late to update it.
-          if (Date.now() > this.lastMtime + STALE_THRESHOLD) {
-            await this.markAsCompromised("Lockfile was not updated in time");
-            return;
-          }
+      // Due to the async nature of this timeout, it's possible that
+      // the lockfile was released before this callback was called.
+      //
+      // Skip the update if that's the case.
+      if (!this.lockfile || this.compromised) {
+        return;
+      }
 
-          // Check if the lockfile got compromised because access was lost or something else updated it.
-          try {
-            const stats = await fs.stat(this.path);
-            if (stats.mtimeMs !== this.lastMtime) {
-              await this.markAsCompromised(
-                "Lockfile was updated by another process"
-              );
-              return;
-            }
-          } catch (error) {
-            await this.markAsCompromised(
-              "Failed to check lockfile status",
-              error
-            );
-            return;
-          }
+      // If we already updated our lockfile once, we need to check if it got compromised.
+      if (this.lastMtime) {
+        // Check if the lockfile got compromised because we were too late to update it.
+        if (Date.now() > this.lastMtime + STALE_THRESHOLD) {
+          this.markAsCompromised("Lockfile was not updated in time");
+          return;
         }
 
-        // Update lockfile's mtime.
+        // Check if the lockfile got compromised because access was lost or something else updated it.
         try {
-          const mtime = new Date(Math.ceil(Date.now() / 1000) * 1000);
-          await fs.utimes(this.path, mtime, mtime);
-          this.lastMtime = mtime.getTime();
+          const stats = fs.statSync(this.path);
+          if (stats.mtimeMs !== this.lastMtime) {
+            this.markAsCompromised("Lockfile was updated by another process");
+            return;
+          }
         } catch (error) {
-          await this.markAsCompromised("Failed to update the lockfile", error);
+          this.markAsCompromised("Failed to check lockfile status", error);
           return;
         }
+      }
+
+      // Update lockfile's mtime.
+      try {
+        const mtime = new Date(Math.ceil(Date.now() / 1000) * 1000);
+        fs.utimesSync(this.path, mtime, mtime);
+        this.lastMtime = mtime.getTime();
+      } catch (error) {
+        this.markAsCompromised("Failed to update the lockfile", error);
+        return;
+      }
 
-        // Lockfile wasn't compromised. Schedule the next update.
-        this.updateLockfileLater();
-      })();
+      // Lockfile wasn't compromised. Schedule the next update.
+      this.updateLockfileLater();
     }, UPDATE_INTERVAL);
   }
 
-  public async release() {
+  public release() {
     if (!this.lockfile) {
-      return; // not locked
+      return;
     }
 
     clearTimeout(this.timeout);
     this.timeout = undefined;
 
-    await this.lockfile.close();
-    await fs.rm(this.path);
+    fs.closeSync(this.lockfile);
     this.lockfile = undefined;
     this.lastMtime = undefined;
-  }
 
-  private async markAsCompromised(reason: string, error?: unknown) {
-    await this.lockfile?.close();
-    this.lockfile = undefined;
-    try {
-      await this.onCompromised?.(reason, error);
-    } catch (callbackError) {
-      console.error(
-        "Unexpected error in Lockfile.onCompromised callback:",
-        callbackError
-      );
+    // If the lockfile was compromised, we won't remove
+    // it since it belongs to another process now.
+    if (!this.compromised) {
+      fs.rmSync(this.path, { force: true });
+      this.compromised = false;
     }
   }
+
+  private markAsCompromised(reason: string, error?: unknown) {
+    this.compromised = true;
+    this.release();
+    void this.onCompromised?.(reason, error);
+  }
 }

libs/wingsdk/src/simulator/simulator.ts

@@ -320,12 +320,12 @@ export class Simulator {
     this._running = "starting";
 
     try {
-      await this.lockfile.lock();
+      this.lockfile.lock();
       await this.startServer();
       await this.startResources();
     } catch (err: any) {
       this.stopServer();
-      await this.lockfile.release();
+      this.lockfile.release();
       this._running = "stopped";
       throw err;
     }
@@ -430,7 +430,7 @@ export class Simulator {
 
     this.stopServer();
 
-    await this.lockfile.release();
+    this.lockfile.release();
 
     this._handles.reset();
     this._running = "stopped";