Race condition in compiler download when more than one Hardhat process is compiling (#4771)

Author: ChristopherDedominici

.changeset/lazy-jobs-turn.md

@@ -0,0 +1,5 @@
+---
+"hardhat": patch
+---
+
+Fixed a race condition that occurred when multiple Hardhat processes ran a compilation at the same time.

packages/hardhat-core/src/internal/solidity/compiler/downloader.ts

@@ -8,7 +8,7 @@ import { promisify } from "util";
 import { download } from "../../util/download";
 import { assertHardhatInvariant, HardhatError } from "../../core/errors";
 import { ERRORS } from "../../core/errors-list";
-import { Mutex } from "../../vendor/await-semaphore";
+import { MultiProcessMutex } from "../../util/multi-process-mutex";
 
 const log = debug("hardhat:core:solidity:downloader");
 
@@ -126,7 +126,7 @@ export class CompilerDownloader implements ICompilerDownloader {
   }
 
   public static defaultCompilerListCachePeriod = 3_600_00;
-  private readonly _mutex = new Mutex();
+  private readonly _mutex = new MultiProcessMutex("compiler-download");
 
   /**
    * Use CompilerDownloader.getConcurrencySafeDownloader instead

packages/hardhat-core/src/internal/util/multi-process-mutex.ts

@@ -0,0 +1,133 @@
+/* eslint-disable @nomicfoundation/hardhat-internal-rules/only-hardhat-error */
+import debug from "debug";
+import fs from "node:fs";
+import path from "node:path";
+import os from "node:os";
+
+// Logic explanation: the fs.writeFile function, when used with the wx+ flag, performs an atomic operation to create a file.
+// If multiple processes try to create the same file simultaneously, only one will succeed.
+// This logic can be utilized to implement a mutex.
+// ATTENTION: in the current implementation, there's still a risk of two processes running simultaneously.
+// For example, if processA has locked the mutex and is running, processB will wait.
+// During this wait, processB continuously checks the elapsed time since the mutex lock file was created.
+// If an excessive amount of time has passed, processB will assume ownership of the mutex to avoid stale locks.
+// However, there's a possibility that processB might take ownership because the mutex creation file is outdated, even though processA is still running
+// For more info check the Nomic Notion page (internal link).
+
+const log = debug("hardhat:util:multi-process-mutex");
+const DEFAULT_MAX_MUTEX_LIFESPAN_IN_MS = 60000;
+const MUTEX_LOOP_WAITING_TIME_IN_MS = 100;
+
+export class MultiProcessMutex {
+  private _mutexFilePath: string;
+  private _mutexLifespanInMs: number;
+
+  constructor(mutexName: string, maxMutexLifespanInMs?: number) {
+    log(`Creating mutex with name '${mutexName}'`);
+
+    this._mutexFilePath = path.join(os.tmpdir(), `${mutexName}.txt`);
+    this._mutexLifespanInMs =
+      maxMutexLifespanInMs ?? DEFAULT_MAX_MUTEX_LIFESPAN_IN_MS;
+  }
+
+  public async use<T>(f: () => Promise<T>): Promise<T> {
+    log(`Starting mutex process with mutex file '${this._mutexFilePath}'`);
+
+    while (true) {
+      if (await this._tryToAcquireMutex()) {
+        // Mutex has been acquired
+        return this._executeFunctionAndReleaseMutex(f);
+      }
+
+      // Mutex not acquired
+      if (this._isMutexFileTooOld()) {
+        // If the mutex file is too old, it likely indicates a stale lock, so the file should be removed
+        log(
+          `Current mutex file is too old, removing it at path '${this._mutexFilePath}'`
+        );
+        this._deleteMutexFile();
+      } else {
+        // wait
+        await this._waitMs();
+      }
+    }
+  }
+
+  private async _tryToAcquireMutex() {
+    try {
+      // Create a file only if it does not exist
+      fs.writeFileSync(this._mutexFilePath, "", { flag: "wx+" });
+      return true;
+    } catch (error: any) {
+      if (error.code === "EEXIST") {
+        // File already exists, so the mutex is already acquired
+        return false;
+      }
+
+      throw error;
+    }
+  }
+
+  private async _executeFunctionAndReleaseMutex<T>(
+    f: () => Promise<T>
+  ): Promise<T> {
+    log(`Mutex acquired at path '${this._mutexFilePath}'`);
+
+    try {
+      const res = await f();
+
+      // Release the mutex
+      log(`Mutex released at path '${this._mutexFilePath}'`);
+      this._deleteMutexFile();
+
+      log(`Mutex released at path '${this._mutexFilePath}'`);
+
+      return res;
+    } catch (error: any) {
+      // Catch any error to avoid stale locks.
+      // Remove the mutex file and re-throw the error
+      this._deleteMutexFile();
+      throw error;
+    }
+  }
+
+  private _isMutexFileTooOld(): boolean {
+    let fileStat;
+    try {
+      fileStat = fs.statSync(this._mutexFilePath);
+    } catch (error: any) {
+      if (error.code === "ENOENT") {
+        // The file might have been deleted by another process while this function was trying to access it.
+        return false;
+      }
+
+      throw error;
+    }
+
+    const now = new Date();
+    const fileDate = new Date(fileStat.ctime);
+    const diff = now.getTime() - fileDate.getTime();
+
+    return diff > this._mutexLifespanInMs;
+  }
+
+  private _deleteMutexFile() {
+    try {
+      log(`Deleting mutex file at path '${this._mutexFilePath}'`);
+      fs.unlinkSync(this._mutexFilePath);
+    } catch (error: any) {
+      if (error.code === "ENOENT") {
+        // The file might have been deleted by another process while this function was trying to access it.
+        return;
+      }
+
+      throw error;
+    }
+  }
+
+  private async _waitMs() {
+    return new Promise((resolve) =>
+      setTimeout(resolve, MUTEX_LOOP_WAITING_TIME_IN_MS)
+    );
+  }
+}

packages/hardhat-core/test/internal/multi-process-mutex.ts

@@ -0,0 +1,98 @@
+import { assert, expect } from "chai";
+import { MultiProcessMutex } from "../../src/internal/util/multi-process-mutex";
+
+describe("multi-process-mutex", () => {
+  const mutexName = "test-mutex";
+
+  it("should execute all the function in a sequential order, not in parallel", async () => {
+    // Since all the functions cannot be executed in parallel because of the mutex,
+    // the total execution time should be bigger than the sum of the execution times of each function.
+    const mutex = new MultiProcessMutex(mutexName); // Use default max mutex lifespan
+    const start = performance.now();
+
+    const ms = [500, 700, 1000];
+    await Promise.all([
+      mutex.use(async () => {
+        await waitMs(ms[0]);
+      }),
+      mutex.use(async () => {
+        await waitMs(ms[1]);
+      }),
+
+      mutex.use(async () => {
+        await waitMs(ms[2]);
+      }),
+    ]);
+
+    const end = performance.now();
+    const duration = end - start;
+
+    expect(duration).to.be.greaterThan(ms[0] + ms[1] + ms[2]);
+  });
+
+  it("should overwrite the current mutex locked by another function because the function took to long to finish", async () => {
+    const mutexLifeSpanMs = 500;
+    const mutex = new MultiProcessMutex(mutexName, mutexLifeSpanMs);
+
+    const res: number[] = [];
+
+    await Promise.all([
+      mutex.use(async () => {
+        await waitMs(2000);
+        res.push(1);
+      }),
+      new Promise((resolve) =>
+        setTimeout(async () => {
+          await mutex.use(async () => {
+            await waitMs(200);
+            res.push(2);
+          });
+          resolve(true);
+        }, 200)
+      ),
+    ]);
+
+    expect(res).to.deep.equal([2, 1]);
+  });
+
+  it("should get the mutex lock because the first function to own it failed", async () => {
+    const mutexLifeSpanMs = 20000; // The mutex should be released and not hit timeout because the first function failed
+    const mutex = new MultiProcessMutex(mutexName, mutexLifeSpanMs);
+
+    const start = performance.now();
+
+    const res: number[] = [];
+    let errThrown = false;
+
+    await Promise.all([
+      mutex
+        .use(async () => {
+          throw new Error("Expected error");
+        })
+        .catch(() => {
+          errThrown = true;
+        }),
+      new Promise((resolve) =>
+        setTimeout(async () => {
+          await mutex.use(async () => {
+            res.push(2);
+          });
+          resolve(true);
+        }, 200)
+      ),
+    ]);
+
+    const end = performance.now();
+    const duration = end - start;
+
+    assert.isTrue(errThrown);
+    expect(res).to.deep.equal([2]);
+    // Since the first function that obtained the mutex failed, the function waiting for it will acquire it instantly.
+    // Therefore, the mutex timeout will not be reached, and the test should complete in less time than the mutex timeout.
+    expect(duration).to.be.lessThan(1000);
+  });
+});
+
+async function waitMs(timeout: number) {
+  return new Promise((resolve) => setTimeout(resolve, timeout));
+}