feat(ngcc): pause async ngcc processing if another process has the lockfile (#35131)

ngcc uses a lockfile to prevent two ngcc instances from executing at the
same time. Previously, if a lockfile was found the current process would
error and exit.

Now, when in async mode, the current process is able to wait for the previous
process to release the lockfile before continuing itself.

PR Close #35131

packages/compiler-cli/ngcc/src/execution/cluster/executor.ts

@@ -13,7 +13,7 @@ import * as cluster from 'cluster';
 import {Logger} from '../../logging/logger';
 import {PackageJsonUpdater} from '../../writing/package_json_updater';
 import {AnalyzeEntryPointsFn, CreateCompileFn, Executor} from '../api';
-import {LockFile} from '../lock_file';
+import {LockFileAsync} from '../lock_file';
 
 import {ClusterMaster} from './master';
 import {ClusterWorker} from './worker';
@@ -26,7 +26,7 @@ import {ClusterWorker} from './worker';
 export class ClusterExecutor implements Executor {
   constructor(
       private workerCount: number, private logger: Logger,
-      private pkgJsonUpdater: PackageJsonUpdater, private lockFile: LockFile) {}
+      private pkgJsonUpdater: PackageJsonUpdater, private lockFile: LockFileAsync) {}
 
   async execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn):
       Promise<void> {

packages/compiler-cli/ngcc/src/execution/lock_file.ts

@@ -6,78 +6,44 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import * as process from 'process';
-import {FileSystem} from '../../../src/ngtsc/file_system';
 
-/**
+import {CachedFileSystem, FileSystem} from '../../../src/ngtsc/file_system';
- * The LockFile is used to prevent more than one instance of ngcc executing at the same time.
+import {Logger} from '../logging/logger';
- *
+
- * When ngcc starts executing, it creates a file in the `compiler-cli/ngcc` folder. If it finds one
+export abstract class LockFileBase {
- * is already there then it fails with a suitable error message.
- * When ngcc completes executing, it removes the file so that future ngcc executions can start.
- */
-export class LockFile {
   lockFilePath =
       this.fs.resolve(require.resolve('@angular/compiler-cli/ngcc'), '../__ngcc_lock_file__');
 
-  constructor(private fs: FileSystem) {}
+  constructor(protected fs: FileSystem) {}
 
-  /**
+  protected writeLockFile(): void {
-   * Run a function guarded by the lock file.
-   *
-   * Note that T can be a Promise. If so, we run the `remove()` call in the promise's `finally`
-   * handler. Otherwise we run the `remove()` call in the `try...finally` block.
-   *
-   * @param fn The function to run.
-   */
-  lock<T>(fn: () => T): T {
-    let isAsync = false;
-    this.create();
     try {
-      const result = fn();
+      this.addSignalHandlers();
-      if (result instanceof Promise) {
+      // To avoid race conditions, we check for existence of the lockfile
-        isAsync = true;
+      // by actually trying to create it exclusively.
-        // The cast is necessary because TS cannot deduce that T is now a promise here.
+      return this.fs.writeFile(this.lockFilePath, process.pid.toString(), /* exclusive */ true);
-        return result.finally(() => this.remove()) as unknown as T;
+    } catch (e) {
-      } else {
+      this.removeSignalHandlers();
-        return result;
+      throw e;
-      }
-    } finally {
-      if (!isAsync) {
-        this.remove();
-      }
     }
   }
 
   /**
-   * Write a lock file to disk, or error if there is already one there.
+   * Read the pid from the lockfile.
+   *
+   * It is feasible that the lockfile was removed between the previous check for existence
+   * and this file-read. If so then we still error but as gracefully as possible.
    */
-  protected create() {
+  protected readLockFile(): string {
     try {
-      this.addSignalHandlers();
+      if (this.fs instanceof CachedFileSystem) {
-      // To avoid race conditions, we check for existence of the lockfile
+        // This file is "volatile", it might be changed by an external process,
-      // by actually trying to create it exclusively
+        // so we cannot rely upon the cached value when reading it.
-      this.fs.writeFile(this.lockFilePath, process.pid.toString(), /* exclusive */ true);
+        this.fs.invalidateCaches(this.lockFilePath);
-    } catch (e) {
-      this.removeSignalHandlers();
-      if (e.code !== 'EEXIST') {
-        throw e;
       }
-
+      return this.fs.readFile(this.lockFilePath);
-      // The lockfile already exists so raise a helpful error.
+    } catch {
-      // It is feasible that the lockfile was removed between the previous check for existence
+      return '{unknown}';
-      // and this file-read. If so then we still error but as gracefully as possible.
-      let pid: string;
-      try {
-        pid = this.fs.readFile(this.lockFilePath);
-      } catch {
-        pid = '{unknown}';
-      }
-
-      throw new Error(
-          `ngcc is already running at process with id ${pid}.\n` +
-          `If you are running multiple builds in parallel then you should pre-process your node_modules via the command line ngcc tool before starting the builds;\n` +
-          `See https://v9.angular.io/guide/ivy#speeding-up-ngcc-compilation.\n` +
-          `(If you are sure no ngcc process is running then you should delete the lockfile at ${this.lockFilePath}.)`);
     }
   }
 
@@ -91,18 +57,25 @@ export class LockFile {
     }
   }
 
+  /**
+   * Capture CTRL-C and terminal closing events.
+   * When these occur we remove the lockfile and exit.
+   */
   protected addSignalHandlers() {
-    process.once('SIGINT', this.signalHandler);
+    process.addListener('SIGINT', this.signalHandler);
-    process.once('SIGHUP', this.signalHandler);
+    process.addListener('SIGHUP', this.signalHandler);
   }
 
+  /**
+   * Clear the event handlers to prevent leakage.
+   */
   protected removeSignalHandlers() {
     process.removeListener('SIGINT', this.signalHandler);
     process.removeListener('SIGHUP', this.signalHandler);
   }
 
   /**
-   * This handle needs to be defined as a property rather than a method
+   * This handler needs to be defined as a property rather than a method
    * so that it can be passed around as a bound function.
    */
   protected signalHandler =
@@ -119,3 +92,113 @@ export class LockFile {
     process.exit(code);
   }
 }
+
+/**
+ * LockFileSync is used to prevent more than one instance of ngcc executing at the same time,
+ * when being called in a synchronous context.
+ *
+ * * When ngcc starts executing, it creates a file in the `compiler-cli/ngcc` folder.
+ * * If it finds one is already there then it fails with a suitable error message.
+ * * When ngcc completes executing, it removes the file so that future ngcc executions can start.
+ */
+export class LockFileSync extends LockFileBase {
+  /**
+   * Run the given function guarded by the lock file.
+   *
+   * @param fn the function to run.
+   * @returns the value returned from the `fn` call.
+   */
+  lock<T>(fn: () => T): T {
+    this.create();
+    try {
+      return fn();
+    } finally {
+      this.remove();
+    }
+  }
+
+  /**
+   * Write a lock file to disk, or error if there is already one there.
+   */
+  protected create(): void {
+    try {
+      this.writeLockFile();
+    } catch (e) {
+      if (e.code !== 'EEXIST') {
+        throw e;
+      }
+      this.handleExistingLockFile();
+    }
+  }
+
+  /**
+   * The lockfile already exists so raise a helpful error.
+   */
+  protected handleExistingLockFile(): void {
+    const pid = this.readLockFile();
+    throw new Error(
+        `ngcc is already running at process with id ${pid}.\n` +
+        `If you are running multiple builds in parallel then you should pre-process your node_modules via the command line ngcc tool before starting the builds;\n` +
+        `See https://v9.angular.io/guide/ivy#speeding-up-ngcc-compilation.\n` +
+        `(If you are sure no ngcc process is running then you should delete the lockfile at ${this.lockFilePath}.)`);
+  }
+}
+
+/**
+ * LockFileAsync is used to prevent more than one instance of ngcc executing at the same time,
+ * when being called in an asynchronous context.
+ *
+ * * When ngcc starts executing, it creates a file in the `compiler-cli/ngcc` folder.
+ * * If it finds one is already there then it pauses and waits for the file to be removed by the
+ *   other process. If the file is not removed within a set timeout period given by
+ *   `retryDelay*retryAttempts` an error is thrown with a suitable error message.
+ * * If the process locking the file changes, then we restart the timeout.
+ * * When ngcc completes executing, it removes the file so that future ngcc executions can start.
+ */
+export class LockFileAsync extends LockFileBase {
+  constructor(
+      fs: FileSystem, protected logger: Logger, private retryDelay: number,
+      private retryAttempts: number) {
+    super(fs);
+  }
+
+  /**
+   * Run a function guarded by the lock file.
+   *
+   * @param fn The function to run.
+   */
+  async lock<T>(fn: () => Promise<T>): Promise<T> {
+    await this.create();
+    return await fn().finally(() => this.remove());
+  }
+
+  protected async create() {
+    let pid: string = '';
+    for (let attempts = 0; attempts < this.retryAttempts; attempts++) {
+      try {
+        return this.writeLockFile();
+      } catch (e) {
+        if (e.code !== 'EEXIST') {
+          throw e;
+        }
+        const newPid = this.readLockFile();
+        if (newPid !== pid) {
+          // The process locking the file has changed, so restart the timeout
+          attempts = 0;
+          pid = newPid;
+        }
+        if (attempts === 0) {
+          this.logger.info(
+              `Another process, with id ${pid}, is currently running ngcc.\n` +
+              `Waiting up to ${this.retryDelay*this.retryAttempts/1000}s for it to finish.`);
+        }
+        // The file is still locked by another process so wait for a bit and retry
+        await new Promise(resolve => setTimeout(resolve, this.retryDelay));
+      }
+    }
+    // If we fall out of the loop then we ran out of rety attempts
+    throw new Error(
+        `Timed out waiting ${this.retryAttempts * this.retryDelay/1000}s for another ngcc process, with id ${pid}, to complete.\n` +
+        `(If you are sure no ngcc process is running then you should delete the lockfile at ${this.lockFilePath}.)`);
+  }
+}

packages/compiler-cli/ngcc/src/execution/single_process_executor.ts

@@ -10,47 +10,56 @@ import {Logger} from '../logging/logger';
 import {PackageJsonUpdater} from '../writing/package_json_updater';
 
 import {AnalyzeEntryPointsFn, CreateCompileFn, Executor} from './api';
-import {LockFile} from './lock_file';
+import {LockFileAsync, LockFileSync} from './lock_file';
 import {onTaskCompleted} from './utils';
 
+export abstract class SingleProcessorExecutorBase {
+  constructor(private logger: Logger, private pkgJsonUpdater: PackageJsonUpdater) {}
+
+  doExecute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn):
+      void|Promise<void> {
+    this.logger.debug(`Running ngcc on ${this.constructor.name}.`);
+
+    const taskQueue = analyzeEntryPoints();
+    const compile =
+        createCompileFn((task, outcome) => onTaskCompleted(this.pkgJsonUpdater, task, outcome));
+
+    // Process all tasks.
+    this.logger.debug('Processing tasks...');
+    const startTime = Date.now();
+
+    while (!taskQueue.allTasksCompleted) {
+      const task = taskQueue.getNextTask() !;
+      compile(task);
+      taskQueue.markTaskCompleted(task);
+    }
+
+    const duration = Math.round((Date.now() - startTime) / 1000);
+    this.logger.debug(`Processed tasks in ${duration}s.`);
+  }
+}
 
 /**
  * An `Executor` that processes all tasks serially and completes synchronously.
  */
-export class SingleProcessExecutor implements Executor {
+export class SingleProcessExecutorSync extends SingleProcessorExecutorBase implements Executor {
-  constructor(
+  constructor(logger: Logger, pkgJsonUpdater: PackageJsonUpdater, private lockfile: LockFileSync) {
-      private logger: Logger, private pkgJsonUpdater: PackageJsonUpdater,
+    super(logger, pkgJsonUpdater);
-      private lockFile: LockFile) {}
+  }
-
   execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn): void {
-    this.lockFile.lock(() => {
+    this.lockfile.lock(() => this.doExecute(analyzeEntryPoints, createCompileFn));
-      this.logger.debug(`Running ngcc on ${this.constructor.name}.`);
-
-      const taskQueue = analyzeEntryPoints();
-      const compile =
-          createCompileFn((task, outcome) => onTaskCompleted(this.pkgJsonUpdater, task, outcome));
-
-      // Process all tasks.
-      this.logger.debug('Processing tasks...');
-      const startTime = Date.now();
-
-      while (!taskQueue.allTasksCompleted) {
-        const task = taskQueue.getNextTask() !;
-        compile(task);
-        taskQueue.markTaskCompleted(task);
-      }
-
-      const duration = Math.round((Date.now() - startTime) / 1000);
-      this.logger.debug(`Processed tasks in ${duration}s.`);
-    });
   }
 }
 
 /**
  * An `Executor` that processes all tasks serially, but still completes asynchronously.
  */
-export class AsyncSingleProcessExecutor extends SingleProcessExecutor {
+export class SingleProcessExecutorAsync extends SingleProcessorExecutorBase implements Executor {
-  async execute(...args: Parameters<Executor['execute']>): Promise<void> {
+  constructor(logger: Logger, pkgJsonUpdater: PackageJsonUpdater, private lockfile: LockFileAsync) {
-    return super.execute(...args);
+    super(logger, pkgJsonUpdater);
+  }
+  async execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn):
+      Promise<void> {
+    await this.lockfile.lock(async() => this.doExecute(analyzeEntryPoints, createCompileFn));
   }
 }

packages/compiler-cli/ngcc/src/main.ts

@@ -27,8 +27,8 @@ import {TargetedEntryPointFinder} from './entry_point_finder/targeted_entry_poin
 import {AnalyzeEntryPointsFn, CreateCompileFn, Executor, PartiallyOrderedTasks, Task, TaskProcessingOutcome, TaskQueue} from './execution/api';
 import {ClusterExecutor} from './execution/cluster/executor';
 import {ClusterPackageJsonUpdater} from './execution/cluster/package_json_updater';
-import {LockFile} from './execution/lock_file';
+import {LockFileAsync, LockFileSync} from './execution/lock_file';
-import {AsyncSingleProcessExecutor, SingleProcessExecutor} from './execution/single_process_executor';
+import {SingleProcessExecutorAsync, SingleProcessExecutorSync} from './execution/single_process_executor';
 import {ParallelTaskQueue} from './execution/task_selection/parallel_task_queue';
 import {SerialTaskQueue} from './execution/task_selection/serial_task_queue';
 import {ConsoleLogger, LogLevel} from './logging/console_logger';
@@ -290,7 +290,7 @@ export function mainNgcc(
   };
 
   // The executor for actually planning and getting the work done.
-  const executor = getExecutor(async, inParallel, logger, pkgJsonUpdater, new LockFile(fileSystem));
+  const executor = getExecutor(async, inParallel, logger, pkgJsonUpdater, fileSystem);
 
   return executor.execute(analyzeEntryPoints, createCompileFn);
 }
@@ -336,16 +336,21 @@ function getTaskQueue(
 
 function getExecutor(
     async: boolean, inParallel: boolean, logger: Logger, pkgJsonUpdater: PackageJsonUpdater,
-    lockFile: LockFile): Executor {
+    fileSystem: FileSystem): Executor {
-  if (inParallel) {
+  if (async) {
-    // Execute in parallel (which implies async).
+    // Execute asynchronously (either serially or in parallel)
-    // Use up to 8 CPU cores for workers, always reserving one for master.
+    const lockFile = new LockFileAsync(fileSystem, logger, 500, 50);
-    const workerCount = Math.min(8, os.cpus().length - 1);
+    if (inParallel) {
-    return new ClusterExecutor(workerCount, logger, pkgJsonUpdater, lockFile);
+      // Execute in parallel. Use up to 8 CPU cores for workers, always reserving one for master.
+      const workerCount = Math.min(8, os.cpus().length - 1);
+      return new ClusterExecutor(workerCount, logger, pkgJsonUpdater, lockFile);
+    } else {
+      // Execute serially, on a single thread (async).
+      return new SingleProcessExecutorAsync(logger, pkgJsonUpdater, lockFile);
+    }
   } else {
-    // Execute serially, on a single thread (either sync or async).
+    // Execute serially, on a single thread (sync).
-    return async ? new AsyncSingleProcessExecutor(logger, pkgJsonUpdater, lockFile) :
+    return new SingleProcessExecutorSync(logger, pkgJsonUpdater, new LockFileSync(fileSystem));
-                   new SingleProcessExecutor(logger, pkgJsonUpdater, lockFile);
   }
 }
 

packages/compiler-cli/ngcc/test/execution/cluster/executor_spec.ts

@@ -14,7 +14,7 @@ import {ClusterExecutor} from '../../../src/execution/cluster/executor';
 import {ClusterMaster} from '../../../src/execution/cluster/master';
 import {ClusterWorker} from '../../../src/execution/cluster/worker';
 import {PackageJsonUpdater} from '../../../src/writing/package_json_updater';
-import {MockLockFile} from '../../helpers/mock_lock_file';
+import {MockLockFileAsync} from '../../helpers/mock_lock_file';
 import {MockLogger} from '../../helpers/mock_logger';
 import {mockProperty} from '../../helpers/spy_utils';
 
@@ -24,7 +24,7 @@ describe('ClusterExecutor', () => {
   let masterRunSpy: jasmine.Spy;
   let workerRunSpy: jasmine.Spy;
   let mockLogger: MockLogger;
-  let mockLockFile: MockLockFile;
+  let mockLockFile: MockLockFileAsync;
   let executor: ClusterExecutor;
 
   beforeEach(() => {
@@ -34,7 +34,7 @@ describe('ClusterExecutor', () => {
                        .and.returnValue(Promise.resolve('CusterWorker#run()'));
 
     mockLogger = new MockLogger();
-    mockLockFile = new MockLockFile();
+    mockLockFile = new MockLockFileAsync();
     executor =
         new ClusterExecutor(42, mockLogger, null as unknown as PackageJsonUpdater, mockLockFile);
   });
@@ -43,9 +43,9 @@ describe('ClusterExecutor', () => {
     describe('(on cluster master)', () => {
       beforeEach(() => runAsClusterMaster(true));
 
-      it('should log debug info about the executor', () => {
+      it('should log debug info about the executor', async() => {
         const anyFn: () => any = () => undefined;
-        executor.execute(anyFn, anyFn);
+        await executor.execute(anyFn, anyFn);
 
         expect(mockLogger.logs.debug).toEqual([
           ['Running ngcc on ClusterExecutor (using 42 worker processes).'],
@@ -88,7 +88,7 @@ describe('ClusterExecutor', () => {
 
       it('should not call master runner if Lockfile.create() fails', async() => {
         const anyFn: () => any = () => undefined;
-        const lockFile = new MockLockFile({throwOnCreate: true});
+        const lockFile = new MockLockFileAsync({throwOnCreate: true});
         executor =
             new ClusterExecutor(42, mockLogger, null as unknown as PackageJsonUpdater, lockFile);
         let error = '';
@@ -104,7 +104,7 @@ describe('ClusterExecutor', () => {
 
       it('should fail if Lockfile.remove() fails', async() => {
         const anyFn: () => any = () => undefined;
-        const lockFile = new MockLockFile({throwOnRemove: true});
+        const lockFile = new MockLockFileAsync({throwOnRemove: true});
         executor =
             new ClusterExecutor(42, mockLogger, null as unknown as PackageJsonUpdater, lockFile);
         let error = '';
@@ -122,9 +122,9 @@ describe('ClusterExecutor', () => {
     describe('(on cluster worker)', () => {
       beforeEach(() => runAsClusterMaster(false));
 
-      it('should not log debug info about the executor', () => {
+      it('should not log debug info about the executor', async() => {
         const anyFn: () => any = () => undefined;
-        executor.execute(anyFn, anyFn);
+        await executor.execute(anyFn, anyFn);
 
         expect(mockLogger.logs.debug).toEqual([]);
       });

packages/compiler-cli/ngcc/test/execution/lock_file_spec.ts

@@ -6,44 +6,130 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import * as process from 'process';
-import {FileSystem, getFileSystem} from '../../../src/ngtsc/file_system';
-import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
-import {LockFile} from '../../src/execution/lock_file';
 
-/**
+import {CachedFileSystem, FileSystem, getFileSystem} from '../../../src/ngtsc/file_system';
- * This class allows us to test the protected methods of LockFile directly,
+import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
- * which are normally hidden as "protected".
+import {LockFileAsync, LockFileBase, LockFileSync} from '../../src/execution/lock_file';
- *
+import {MockLogger} from '../helpers/mock_logger';
- * We also add logging in here to track what is being called and in what order.
- *
- * Finally this class stubs out the `exit()` method to prevent unit tests from exiting the process.
- */
-class LockFileUnderTest extends LockFile {
-  log: string[] = [];
-  constructor(fs: FileSystem, private handleSignals = false) {
-    super(fs);
-    fs.ensureDir(fs.dirname(this.lockFilePath));
-  }
-  create() {
-    this.log.push('create()');
-    super.create();
-  }
-  remove() {
-    this.log.push('remove()');
-    super.remove();
-  }
-  addSignalHandlers() {
-    if (this.handleSignals) {
-      super.addSignalHandlers();
-    }
-  }
-  removeSignalHandlers() { super.removeSignalHandlers(); }
-  exit(code: number) { this.log.push(`exit(${code})`); }
-}
 
 runInEachFileSystem(() => {
-  describe('LockFile', () => {
+  describe('LockFileBase', () => {
-    describe('lock() - synchronous', () => {
+    /**
+     * This class allows us to test the abstract class LockFileBase.
+     */
+    class LockFileUnderTest extends LockFileBase {
+      log: string[] = [];
+      constructor(fs: FileSystem, private handleSignals = false) {
+        super(fs);
+        fs.ensureDir(fs.dirname(this.lockFilePath));
+      }
+      remove() { super.remove(); }
+      addSignalHandlers() {
+        this.log.push('addSignalHandlers()');
+        if (this.handleSignals) {
+          super.addSignalHandlers();
+        }
+      }
+      writeLockFile() { super.writeLockFile(); }
+      readLockFile() { return super.readLockFile(); }
+      removeSignalHandlers() {
+        this.log.push('removeSignalHandlers()');
+        super.removeSignalHandlers();
+      }
+      exit(code: number) { this.log.push(`exit(${code})`); }
+    }
+
+    describe('writeLockFile()', () => {
+      it('should call `addSignalHandlers()`', () => {
+        const fs = getFileSystem();
+        const lockFile = new LockFileUnderTest(fs);
+        lockFile.writeLockFile();
+        expect(lockFile.log).toEqual(['addSignalHandlers()']);
+      });
+
+      it('should call `removeSignalHandlers()` if there is an error', () => {
+        const fs = getFileSystem();
+        spyOn(fs, 'writeFile').and.throwError('WRITING ERROR');
+        const lockFile = new LockFileUnderTest(fs);
+        expect(() => lockFile.writeLockFile()).toThrowError('WRITING ERROR');
+        expect(lockFile.log).toEqual(['addSignalHandlers()', 'removeSignalHandlers()']);
+      });
+    });
+
+    describe('readLockFile()', () => {
+      it('should return the contents of the lockfile', () => {
+        const fs = getFileSystem();
+        const lockFile = new LockFileUnderTest(fs);
+        fs.writeFile(lockFile.lockFilePath, '188');
+        expect(lockFile.readLockFile()).toEqual('188');
+      });
+
+      it('should return `{unknown}` if the lockfile does not exist', () => {
+        const fs = getFileSystem();
+        const lockFile = new LockFileUnderTest(fs);
+        expect(lockFile.readLockFile()).toEqual('{unknown}');
+      });
+    });
+
+    describe('remove()', () => {
+      it('should remove the lock file from the file-system', () => {
+        const fs = getFileSystem();
+        const lockFile = new LockFileUnderTest(fs);
+        fs.writeFile(lockFile.lockFilePath, '188');
+        lockFile.remove();
+        expect(fs.exists(lockFile.lockFilePath)).toBe(false);
+      });
+
+      it('should not error if the lock file does not exist', () => {
+        const fs = getFileSystem();
+        const lockFile = new LockFileUnderTest(fs);
+        expect(() => lockFile.remove()).not.toThrow();
+      });
+
+      it('should call removeSignalHandlers()', () => {
+        const fs = getFileSystem();
+        const lockFile = new LockFileUnderTest(fs);
+        fs.writeFile(lockFile.lockFilePath, '188');
+        lockFile.remove();
+        expect(lockFile.log).toEqual(['removeSignalHandlers()']);
+      });
+    });
+  });
+
+  describe('LockFileSync', () => {
+    /**
+     * This class allows us to test the protected methods of LockFileSync directly,
+     * which are normally hidden as "protected".
+     *
+     * We also add logging in here to track what is being called and in what order.
+     *
+     * Finally this class stubs out the `exit()` method to prevent unit tests from exiting the
+     * process.
+     */
+    class LockFileUnderTest extends LockFileSync {
+      log: string[] = [];
+      constructor(fs: FileSystem, private handleSignals = false) {
+        super(fs);
+        fs.ensureDir(fs.dirname(this.lockFilePath));
+      }
+      create() {
+        this.log.push('create()');
+        super.create();
+      }
+      remove() {
+        this.log.push('remove()');
+        super.remove();
+      }
+      addSignalHandlers() {
+        if (this.handleSignals) {
+          super.addSignalHandlers();
+        }
+      }
+      removeSignalHandlers() { super.removeSignalHandlers(); }
+      exit(code: number) { this.log.push(`exit(${code})`); }
+    }
+
+    describe('lock()', () => {
       it('should guard the `fn()` with calls to `create()` and `remove()`', () => {
         const fs = getFileSystem();
         const lockFile = new LockFileUnderTest(fs);
@@ -101,72 +187,6 @@ runInEachFileSystem(() => {
       });
     });
 
-    describe('lock() - asynchronous', () => {
-      it('should guard the `fn()` with calls to `create()` and `remove()`', async() => {
-        const fs = getFileSystem();
-        const lockFile = new LockFileUnderTest(fs);
-
-        await lockFile.lock(async() => {
-          lockFile.log.push('fn() - before');
-          // This promise forces node to do a tick in this function, ensuring that we are truly
-          // testing an async scenario.
-          await Promise.resolve();
-          lockFile.log.push('fn() - after');
-        });
-        expect(lockFile.log).toEqual(['create()', 'fn() - before', 'fn() - after', 'remove()']);
-      });
-
-      it('should guard the `fn()` with calls to `create()` and `remove()`, even if it throws',
-         async() => {
-           let error: string = '';
-           const fs = getFileSystem();
-           const lockFile = new LockFileUnderTest(fs);
-           lockFile.create = () => lockFile.log.push('create()');
-           lockFile.remove = () => lockFile.log.push('remove()');
-
-           try {
-             await lockFile.lock(async() => {
-               lockFile.log.push('fn()');
-               throw new Error('ERROR');
-             });
-           } catch (e) {
-             error = e.message;
-           }
-           expect(error).toEqual('ERROR');
-           expect(lockFile.log).toEqual(['create()', 'fn()', 'remove()']);
-         });
-
-      it('should remove the lockfile if CTRL-C is triggered', async() => {
-        const fs = getFileSystem();
-        const lockFile = new LockFileUnderTest(fs, /* handleSignals */ true);
-
-        await lockFile.lock(async() => {
-          lockFile.log.push('SIGINT');
-          process.emit('SIGINT', 'SIGINT');
-        });
-        // Since the test does not actually exit process, the `remove()` is called one more time.
-        expect(lockFile.log).toEqual(['create()', 'SIGINT', 'remove()', 'exit(1)', 'remove()']);
-        // Clean up the signal handlers. In practice this is not needed since the process would have
-        // been terminated already.
-        lockFile.removeSignalHandlers();
-      });
-
-      it('should remove the lockfile if terminal is closed', async() => {
-        const fs = getFileSystem();
-        const lockFile = new LockFileUnderTest(fs, /* handleSignals */ true);
-
-        await lockFile.lock(async() => {
-          lockFile.log.push('SIGHUP');
-          process.emit('SIGHUP', 'SIGHUP');
-        });
-        // Since this does not actually exit process, the `remove()` is called one more time.
-        expect(lockFile.log).toEqual(['create()', 'SIGHUP', 'remove()', 'exit(1)', 'remove()']);
-        // Clean up the signal handlers. In practice this is not needed since the process would have
-        // been terminated already.
-        lockFile.removeSignalHandlers();
-      });
-    });
-
     describe('create()', () => {
       it('should write a lock file to the file-system', () => {
         const fs = getFileSystem();
@@ -188,21 +208,180 @@ runInEachFileSystem(() => {
                 `(If you are sure no ngcc process is running then you should delete the lockfile at ${lockFile.lockFilePath}.)`);
       });
     });
+  });
 
-    describe('remove()', () => {
+  describe('LockFileAsync', () => {
-      it('should remove the lock file from the file-system', () => {
+    /**
+     * This class allows us to test the protected methods of LockFileAsync directly,
+     * which are normally hidden as "protected".
+     *
+     * We also add logging in here to track what is being called and in what order.
+     *
+     * Finally this class stubs out the `exit()` method to prevent unit tests from exiting the
+     * process.
+     */
+    class LockFileUnderTest extends LockFileAsync {
+      log: string[] = [];
+      constructor(
+          fs: FileSystem, retryDelay = 100, retryAttempts = 10, private handleSignals = false) {
+        super(fs, new MockLogger(), retryDelay, retryAttempts);
+        fs.ensureDir(fs.dirname(this.lockFilePath));
+      }
+      async create() {
+        this.log.push('create()');
+        await super.create();
+      }
+      remove() {
+        this.log.push('remove()');
+        super.remove();
+      }
+      addSignalHandlers() {
+        if (this.handleSignals) {
+          super.addSignalHandlers();
+        }
+      }
+      removeSignalHandlers() { super.removeSignalHandlers(); }
+      exit(code: number) { this.log.push(`exit(${code})`); }
+      getLogger() { return this.logger as MockLogger; }
+    }
+
+    describe('lock()', () => {
+      it('should guard the `fn()` with calls to `create()` and `remove()`', async() => {
+        const fs = getFileSystem();
+        const lockFile = new LockFileUnderTest(fs);
+
+        await lockFile.lock(async() => {
+          lockFile.log.push('fn() - before');
+          // This promise forces node to do a tick in this function, ensuring that we are truly
+          // testing an async scenario.
+          await Promise.resolve();
+          lockFile.log.push('fn() - after');
+          return Promise.resolve();
+        });
+        expect(lockFile.log).toEqual(['create()', 'fn() - before', 'fn() - after', 'remove()']);
+      });
+
+      it('should guard the `fn()` with calls to `create()` and `remove()`, even if it throws',
+         async() => {
+           let error: string = '';
+           const fs = getFileSystem();
+           const lockFile = new LockFileUnderTest(fs);
+           try {
+             await lockFile.lock(async() => {
+               lockFile.log.push('fn()');
+               throw new Error('ERROR');
+             });
+           } catch (e) {
+             error = e.message;
+           }
+           expect(error).toEqual('ERROR');
+           expect(lockFile.log).toEqual(['create()', 'fn()', 'remove()']);
+         });
+
+      it('should remove the lockfile if CTRL-C is triggered', async() => {
+        const fs = getFileSystem();
+        const lockFile = new LockFileUnderTest(fs, 100, 3, /* handleSignals */ true);
+
+        await lockFile.lock(async() => {
+          lockFile.log.push('SIGINT');
+          process.emit('SIGINT', 'SIGINT');
+        });
+        // Since the test does not actually exit process, the `remove()` is called one more time.
+        expect(lockFile.log).toEqual(['create()', 'SIGINT', 'remove()', 'exit(1)', 'remove()']);
+        // Clean up the signal handlers. In practice this is not needed since the process would have
+        // been terminated already.
+        lockFile.removeSignalHandlers();
+      });
+
+      it('should remove the lockfile if terminal is closed', async() => {
+        const fs = getFileSystem();
+        const lockFile = new LockFileUnderTest(fs, 100, 3, /* handleSignals */ true);
+
+        await lockFile.lock(async() => {
+          lockFile.log.push('SIGHUP');
+          process.emit('SIGHUP', 'SIGHUP');
+        });
+        // Since this does not actually exit process, the `remove()` is called one more time.
+        expect(lockFile.log).toEqual(['create()', 'SIGHUP', 'remove()', 'exit(1)', 'remove()']);
+        // Clean up the signal handlers. In practice this is not needed since the process would have
+        // been terminated already.
+        lockFile.removeSignalHandlers();
+      });
+    });
+
+    describe('create()', () => {
+      it('should write a lock file to the file-system', async() => {
+        const fs = getFileSystem();
+        const lockFile = new LockFileUnderTest(fs);
+        expect(fs.exists(lockFile.lockFilePath)).toBe(false);
+        await lockFile.create();
+        expect(fs.exists(lockFile.lockFilePath)).toBe(true);
+      });
+
+      it('should retry if another process is locking', async() => {
         const fs = getFileSystem();
         const lockFile = new LockFileUnderTest(fs);
         fs.writeFile(lockFile.lockFilePath, '188');
-        lockFile.remove();
+        const promise = lockFile.lock(async() => lockFile.log.push('fn()'));
-        expect(fs.exists(lockFile.lockFilePath)).toBe(false);
+        // The lock is now waiting on the lockfile becoming free, so no `fn()` in the log.
+        expect(lockFile.log).toEqual(['create()']);
+        expect(lockFile.getLogger().logs.info).toEqual([[
+          'Another process, with id 188, is currently running ngcc.\nWaiting up to 1s for it to finish.'
+        ]]);
+        fs.removeFile(lockFile.lockFilePath);
+        // The lockfile has been removed, so we can create our own lockfile, call `fn()` and then
+        // remove the lockfile.
+        await promise;
+        expect(lockFile.log).toEqual(['create()', 'fn()', 'remove()']);
       });
 
-      it('should not error if the lock file does not exist', () => {
+      it('should extend the retry timeout if the other process locking the file changes', async() => {
-        const fs = getFileSystem();
+        // Use a cached file system to test that we are invalidating it correctly
+        const rawFs = getFileSystem();
+        const fs = new CachedFileSystem(rawFs);
         const lockFile = new LockFileUnderTest(fs);
-        expect(() => lockFile.remove()).not.toThrow();
+        fs.writeFile(lockFile.lockFilePath, '188');
+        const promise = lockFile.lock(async() => lockFile.log.push('fn()'));
+        // The lock is now waiting on the lockfile becoming free, so no `fn()` in the log.
+        expect(lockFile.log).toEqual(['create()']);
+        expect(lockFile.getLogger().logs.info).toEqual([[
+          'Another process, with id 188, is currently running ngcc.\nWaiting up to 1s for it to finish.'
+        ]]);
+        // We need to write to the rawFs to ensure that we don't update the cache at this point
+        rawFs.writeFile(lockFile.lockFilePath, '444');
+        await new Promise(resolve => setTimeout(resolve, 250));
+        expect(lockFile.getLogger().logs.info).toEqual([
+          [
+            'Another process, with id 188, is currently running ngcc.\nWaiting up to 1s for it to finish.'
+          ],
+          [
+            'Another process, with id 444, is currently running ngcc.\nWaiting up to 1s for it to finish.'
+          ]
+        ]);
+        fs.removeFile(lockFile.lockFilePath);
+        // The lockfile has been removed, so we can create our own lockfile, call `fn()` and then
+        // remove the lockfile.
+        await promise;
+        expect(lockFile.log).toEqual(['create()', 'fn()', 'remove()']);
       });
+
+      it('should error if another process does not release the lockfile before this times out',
+         async() => {
+           const fs = getFileSystem();
+           const lockFile = new LockFileUnderTest(fs, 100, 2);
+           fs.writeFile(lockFile.lockFilePath, '188');
+           const promise = lockFile.lock(async() => lockFile.log.push('fn()'));
+           // The lock is now waiting on the lockfile becoming free, so no `fn()` in the log.
+           expect(lockFile.log).toEqual(['create()']);
+           // Do not remove the lockfile and let the call to `lock()` timeout.
+           let error: Error;
+           await promise.catch(e => error = e);
+           expect(lockFile.log).toEqual(['create()']);
+           expect(error !.message)
+               .toEqual(
+                   `Timed out waiting 0.2s for another ngcc process, with id 188, to complete.\n` +
+                   `(If you are sure no ngcc process is running then you should delete the lockfile at ${lockFile.lockFilePath}.)`);
+         });
     });
   });
 });

packages/compiler-cli/ngcc/test/execution/single_processor_executor_spec.ts

@@ -8,23 +8,23 @@
 
 /// <reference types="node" />
 
-import {SingleProcessExecutor} from '../../src/execution/single_process_executor';
+import {SingleProcessExecutorSync} from '../../src/execution/single_process_executor';
 import {SerialTaskQueue} from '../../src/execution/task_selection/serial_task_queue';
 import {PackageJsonUpdater} from '../../src/writing/package_json_updater';
-import {MockLockFile} from '../helpers/mock_lock_file';
+import {MockLockFileSync} from '../helpers/mock_lock_file';
 import {MockLogger} from '../helpers/mock_logger';
 
 
 describe('SingleProcessExecutor', () => {
   let mockLogger: MockLogger;
-  let mockLockFile: MockLockFile;
+  let mockLockFile: MockLockFileSync;
-  let executor: SingleProcessExecutor;
+  let executor: SingleProcessExecutorSync;
 
   beforeEach(() => {
     mockLogger = new MockLogger();
-    mockLockFile = new MockLockFile();
+    mockLockFile = new MockLockFileSync();
-    executor =
+    executor = new SingleProcessExecutorSync(
-        new SingleProcessExecutor(mockLogger, null as unknown as PackageJsonUpdater, mockLockFile);
+        mockLogger, null as unknown as PackageJsonUpdater, mockLockFile);
   });
 
   describe('execute()', () => {
@@ -63,11 +63,11 @@ describe('SingleProcessExecutor', () => {
     });
 
     it('should not call `analyzeEntryPoints` if Lockfile.create() fails', () => {
-      const lockFile = new MockLockFile({throwOnCreate: true});
+      const lockFile = new MockLockFileSync({throwOnCreate: true});
       const analyzeFn: () => any = () => { lockFile.log.push('analyzeFn'); };
       const anyFn: () => any = () => undefined;
-      executor =
+      executor = new SingleProcessExecutorSync(
-          new SingleProcessExecutor(mockLogger, null as unknown as PackageJsonUpdater, lockFile);
+          mockLogger, null as unknown as PackageJsonUpdater, lockFile);
       let error = '';
       try {
         executor.execute(analyzeFn, anyFn);
@@ -81,9 +81,9 @@ describe('SingleProcessExecutor', () => {
     it('should fail if Lockfile.remove() fails', () => {
       const noTasks = () => new SerialTaskQueue([] as any);
       const anyFn: () => any = () => undefined;
-      const lockFile = new MockLockFile({throwOnRemove: true});
+      const lockFile = new MockLockFileSync({throwOnRemove: true});
-      executor =
+      executor = new SingleProcessExecutorSync(
-          new SingleProcessExecutor(mockLogger, null as unknown as PackageJsonUpdater, lockFile);
+          mockLogger, null as unknown as PackageJsonUpdater, lockFile);
       let error = '';
       try {
         executor.execute(noTasks, anyFn);

packages/compiler-cli/ngcc/test/helpers/mock_lock_file.ts

@@ -6,9 +6,10 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import {MockFileSystemNative} from '../../../src/ngtsc/file_system/testing';
-import {LockFile} from '../../src/execution/lock_file';
+import {LockFileAsync, LockFileSync} from '../../src/execution/lock_file';
+import {MockLogger} from './mock_logger';
 
-export class MockLockFile extends LockFile {
+export class MockLockFileSync extends LockFileSync {
   log: string[] = [];
   constructor(private options: {throwOnCreate?: boolean, throwOnRemove?: boolean} = {}) {
     // This `MockLockFile` is not used in tests that are run via `runInEachFileSystem()`
@@ -24,3 +25,20 @@ export class MockLockFile extends LockFile {
     if (this.options.throwOnRemove) throw new Error('LockFile.remove() error');
   }
 }
+
+export class MockLockFileAsync extends LockFileAsync {
+  log: string[] = [];
+  constructor(private options: {throwOnCreate?: boolean, throwOnRemove?: boolean} = {}) {
+    // This `MockLockFile` is not used in tests that are run via `runInEachFileSystem()`
+    // So we cannot use `getFileSystem()` but instead just instantiate a mock file-system.
+    super(new MockFileSystemNative(), new MockLogger(), 200, 2);
+  }
+  async create() {
+    this.log.push('create()');
+    if (this.options.throwOnCreate) throw new Error('LockFile.create() error');
+  }
+  remove() {
+    this.log.push('remove()');
+    if (this.options.throwOnRemove) throw new Error('LockFile.remove() error');
+  }
+}

packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts

@@ -13,7 +13,7 @@ import * as os from 'os';
 import {AbsoluteFsPath, FileSystem, absoluteFrom, getFileSystem, join} from '../../../src/ngtsc/file_system';
 import {Folder, MockFileSystem, TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
 import {loadStandardTestFiles, loadTestFiles} from '../../../test/helpers';
-import {LockFile} from '../../src/execution/lock_file';
+import {LockFileSync} from '../../src/execution/lock_file';
 import {mainNgcc} from '../../src/main';
 import {markAsProcessed} from '../../src/packages/build_marker';
 import {EntryPointJsonProperty, EntryPointPackageJson, SUPPORTED_FORMAT_PROPERTIES} from '../../src/packages/entry_point';
@@ -1507,7 +1507,7 @@ runInEachFileSystem(() => {
     function initMockFileSystem(fs: FileSystem, testFiles: Folder) {
       if (fs instanceof MockFileSystem) {
         fs.init(testFiles);
-        fs.ensureDir(fs.dirname(new LockFile(fs).lockFilePath));
+        fs.ensureDir(fs.dirname(new LockFileSync(fs).lockFilePath));
       }
 
       // a random test package that no metadata.json file so not compiled by Angular.

packages/compiler-cli/src/ngtsc/file_system/test/cached_file_system_spec.ts

@@ -91,7 +91,7 @@ describe('CachedFileSystem', () => {
   });
 
   describe('invalidateCaches()', () => {
-    it('should use the delegate for `readFile()` if the path for the cached file has been invalidated',
+    it('should call the delegate `readFile()` if the path for the cached file has been invalidated',
        () => {
          spyOn(delegate, 'lstat').and.returnValue({isSymbolicLink: () => false});
          const spy = spyOn(delegate, 'readFile').and.returnValue('Some contents');
@@ -107,7 +107,7 @@ describe('CachedFileSystem', () => {
          expect(spy).toHaveBeenCalledWith(abcPath);
        });
 
-    it('should use the delegate `exists()` if the path for the cached file has been invalidated',
+    it('should call the delegate `exists()` if the path for the cached file has been invalidated',
        () => {
          const spy = spyOn(delegate, 'exists').and.returnValue(true);
          fs.exists(abcPath);  // Call once to fill the cache