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

This reverts commit b970028057.

This is a feature commit and was improperly merged to the patch branch.

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 {LockFileAsync} from '../lock_file';
+import {LockFile} 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: LockFileAsync) {}
+      private pkgJsonUpdater: PackageJsonUpdater, private lockFile: LockFile) {}
 
   async execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn):
       Promise<void> {

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

@@ -6,44 +6,78 @@
  * 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';
+/**
-import {Logger} from '../logging/logger';
+ * The LockFile is used to prevent more than one instance of ngcc executing at the same time.
-
+ *
-export abstract class LockFileBase {
+ * 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 LockFile {
   lockFilePath =
       this.fs.resolve(require.resolve('@angular/compiler-cli/ngcc'), '../__ngcc_lock_file__');
 
-  constructor(protected fs: FileSystem) {}
+  constructor(private 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 {
-      this.addSignalHandlers();
+      const result = fn();
-      // To avoid race conditions, we check for existence of the lockfile
+      if (result instanceof Promise) {
-      // by actually trying to create it exclusively.
+        isAsync = true;
-      return this.fs.writeFile(this.lockFilePath, process.pid.toString(), /* exclusive */ true);
+        // The cast is necessary because TS cannot deduce that T is now a promise here.
-    } catch (e) {
+        return result.finally(() => this.remove()) as unknown as T;
-      this.removeSignalHandlers();
+      } else {
-      throw e;
+        return result;
+      }
+    } finally {
+      if (!isAsync) {
+        this.remove();
+      }
     }
   }
 
   /**
-   * Read the pid from the lockfile.
+   * Write a lock file to disk, or error if there is already one there.
-   *
-   * 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 readLockFile(): string {
+  protected create() {
     try {
-      if (this.fs instanceof CachedFileSystem) {
+      this.addSignalHandlers();
-        // This file is "volatile", it might be changed by an external process,
+      // To avoid race conditions, we check for existence of the lockfile
-        // so we cannot rely upon the cached value when reading it.
+      // by actually trying to create it exclusively
-        this.fs.invalidateCaches(this.lockFilePath);
+      this.fs.writeFile(this.lockFilePath, process.pid.toString(), /* exclusive */ true);
+    } 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.
+      // 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.
+      let pid: string;
+      try {
+        pid = this.fs.readFile(this.lockFilePath);
       } catch {
-      return '{unknown}';
+        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}.)`);
     }
   }
 
@@ -57,25 +91,18 @@ export abstract class LockFileBase {
     }
   }
 
-  /**
-   * Capture CTRL-C and terminal closing events.
-   * When these occur we remove the lockfile and exit.
-   */
   protected addSignalHandlers() {
-    process.addListener('SIGINT', this.signalHandler);
+    process.once('SIGINT', this.signalHandler);
-    process.addListener('SIGHUP', this.signalHandler);
+    process.once('SIGHUP', this.signalHandler);
   }
 
-  /**
-   * Clear the event handlers to prevent leakage.
-   */
   protected removeSignalHandlers() {
     process.removeListener('SIGINT', this.signalHandler);
     process.removeListener('SIGHUP', this.signalHandler);
   }
 
   /**
-   * This handler needs to be defined as a property rather than a method
+   * This handle needs to be defined as a property rather than a method
    * so that it can be passed around as a bound function.
    */
   protected signalHandler =
@@ -92,113 +119,3 @@ export abstract class LockFileBase {
     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,14 +10,20 @@ import {Logger} from '../logging/logger';
 import {PackageJsonUpdater} from '../writing/package_json_updater';
 
 import {AnalyzeEntryPointsFn, CreateCompileFn, Executor} from './api';
-import {LockFileAsync, LockFileSync} from './lock_file';
+import {LockFile} 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> {
+ * An `Executor` that processes all tasks serially and completes synchronously.
+ */
+export class SingleProcessExecutor implements Executor {
+  constructor(
+      private logger: Logger, private pkgJsonUpdater: PackageJsonUpdater,
+      private lockFile: LockFile) {}
+
+  execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn): void {
+    this.lockFile.lock(() => {
       this.logger.debug(`Running ngcc on ${this.constructor.name}.`);
 
       const taskQueue = analyzeEntryPoints();
@@ -36,30 +42,15 @@ export abstract class SingleProcessorExecutorBase {
 
       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 SingleProcessExecutorSync extends SingleProcessorExecutorBase implements Executor {
-  constructor(logger: Logger, pkgJsonUpdater: PackageJsonUpdater, private lockfile: LockFileSync) {
-    super(logger, pkgJsonUpdater);
-  }
-  execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn): void {
-    this.lockfile.lock(() => this.doExecute(analyzeEntryPoints, createCompileFn));
   }
 }
 
 /**
  * An `Executor` that processes all tasks serially, but still completes asynchronously.
  */
-export class SingleProcessExecutorAsync extends SingleProcessorExecutorBase implements Executor {
+export class AsyncSingleProcessExecutor extends SingleProcessExecutor {
-  constructor(logger: Logger, pkgJsonUpdater: PackageJsonUpdater, private lockfile: LockFileAsync) {
+  async execute(...args: Parameters<Executor['execute']>): Promise<void> {
-    super(logger, pkgJsonUpdater);
+    return super.execute(...args);
-  }
-  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 {LockFileAsync, LockFileSync} from './execution/lock_file';
+import {LockFile} from './execution/lock_file';
-import {SingleProcessExecutorAsync, SingleProcessExecutorSync} from './execution/single_process_executor';
+import {AsyncSingleProcessExecutor, SingleProcessExecutor} 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, fileSystem);
+  const executor = getExecutor(async, inParallel, logger, pkgJsonUpdater, new LockFile(fileSystem));
 
   return executor.execute(analyzeEntryPoints, createCompileFn);
 }
@@ -336,21 +336,16 @@ function getTaskQueue(
 
 function getExecutor(
     async: boolean, inParallel: boolean, logger: Logger, pkgJsonUpdater: PackageJsonUpdater,
-    fileSystem: FileSystem): Executor {
+    lockFile: LockFile): Executor {
-  if (async) {
-    // Execute asynchronously (either serially or in parallel)
-    const lockFile = new LockFileAsync(fileSystem, logger, 500, 50);
   if (inParallel) {
-      // Execute in parallel. Use up to 8 CPU cores for workers, always reserving one for master.
+    // Execute in parallel (which implies async).
+    // 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).
+    // Execute serially, on a single thread (either sync or async).
-      return new SingleProcessExecutorAsync(logger, pkgJsonUpdater, lockFile);
+    return async ? new AsyncSingleProcessExecutor(logger, pkgJsonUpdater, lockFile) :
-    }
+                   new SingleProcessExecutor(logger, pkgJsonUpdater, lockFile);
-  } else {
-    // Execute serially, on a single thread (sync).
-    return new SingleProcessExecutorSync(logger, pkgJsonUpdater, new LockFileSync(fileSystem));
   }
 }
 

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 {MockLockFileAsync} from '../../helpers/mock_lock_file';
+import {MockLockFile} 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: MockLockFileAsync;
+  let mockLockFile: MockLockFile;
   let executor: ClusterExecutor;
 
   beforeEach(() => {
@@ -34,7 +34,7 @@ describe('ClusterExecutor', () => {
                        .and.returnValue(Promise.resolve('CusterWorker#run()'));
 
     mockLogger = new MockLogger();
-    mockLockFile = new MockLockFileAsync();
+    mockLockFile = new MockLockFile();
     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', async() => {
+      it('should log debug info about the executor', () => {
         const anyFn: () => any = () => undefined;
-        await executor.execute(anyFn, anyFn);
+        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 MockLockFileAsync({throwOnCreate: true});
+        const lockFile = new MockLockFile({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 MockLockFileAsync({throwOnRemove: true});
+        const lockFile = new MockLockFile({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', async() => {
+      it('should not log debug info about the executor', () => {
         const anyFn: () => any = () => undefined;
-        await executor.execute(anyFn, anyFn);
+        executor.execute(anyFn, anyFn);
 
         expect(mockLogger.logs.debug).toEqual([]);
       });

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

@@ -6,107 +6,19 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import * as process from 'process';
-
+import {FileSystem, getFileSystem} from '../../../src/ngtsc/file_system';
-import {CachedFileSystem, FileSystem, getFileSystem} from '../../../src/ngtsc/file_system';
 import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
-import {LockFileAsync, LockFileBase, LockFileSync} from '../../src/execution/lock_file';
+import {LockFile} from '../../src/execution/lock_file';
-import {MockLogger} from '../helpers/mock_logger';
 
-runInEachFileSystem(() => {
+/**
-  describe('LockFileBase', () => {
+ * This class allows us to test the protected methods of LockFile directly,
-    /**
-     * 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
+ * Finally this class stubs out the `exit()` method to prevent unit tests from exiting the process.
-     * process.
  */
-    class LockFileUnderTest extends LockFileSync {
+class LockFileUnderTest extends LockFile {
   log: string[] = [];
   constructor(fs: FileSystem, private handleSignals = false) {
     super(fs);
@@ -127,9 +39,11 @@ runInEachFileSystem(() => {
   }
   removeSignalHandlers() { super.removeSignalHandlers(); }
   exit(code: number) { this.log.push(`exit(${code})`); }
-    }
+}
 
-    describe('lock()', () => {
+runInEachFileSystem(() => {
+  describe('LockFile', () => {
+    describe('lock() - synchronous', () => {
       it('should guard the `fn()` with calls to `create()` and `remove()`', () => {
         const fs = getFileSystem();
         const lockFile = new LockFileUnderTest(fs);
@@ -187,6 +101,72 @@ 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();
@@ -208,179 +188,20 @@ runInEachFileSystem(() => {
                 `(If you are sure no ngcc process is running then you should delete the lockfile at ${lockFile.lockFilePath}.)`);
       });
     });
-  });
 
-  describe('LockFileAsync', () => {
+    describe('remove()', () => {
-    /**
+      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);
+        fs.writeFile(lockFile.lockFilePath, '188');
+        lockFile.remove();
         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() => {
+      it('should not error if the lock file does not exist', () => {
         const fs = getFileSystem();
         const lockFile = new LockFileUnderTest(fs);
-        fs.writeFile(lockFile.lockFilePath, '188');
+        expect(() => lockFile.remove()).not.toThrow();
-        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.'
-        ]]);
-        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 extend the retry timeout if the other process locking the file changes', async() => {
-        // 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);
-        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 {SingleProcessExecutorSync} from '../../src/execution/single_process_executor';
+import {SingleProcessExecutor} 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 {MockLockFileSync} from '../helpers/mock_lock_file';
+import {MockLockFile} from '../helpers/mock_lock_file';
 import {MockLogger} from '../helpers/mock_logger';
 
 
 describe('SingleProcessExecutor', () => {
   let mockLogger: MockLogger;
-  let mockLockFile: MockLockFileSync;
+  let mockLockFile: MockLockFile;
-  let executor: SingleProcessExecutorSync;
+  let executor: SingleProcessExecutor;
 
   beforeEach(() => {
     mockLogger = new MockLogger();
-    mockLockFile = new MockLockFileSync();
+    mockLockFile = new MockLockFile();
-    executor = new SingleProcessExecutorSync(
+    executor =
-        mockLogger, null as unknown as PackageJsonUpdater, mockLockFile);
+        new SingleProcessExecutor(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 MockLockFileSync({throwOnCreate: true});
+      const lockFile = new MockLockFile({throwOnCreate: true});
       const analyzeFn: () => any = () => { lockFile.log.push('analyzeFn'); };
       const anyFn: () => any = () => undefined;
-      executor = new SingleProcessExecutorSync(
+      executor =
-          mockLogger, null as unknown as PackageJsonUpdater, lockFile);
+          new SingleProcessExecutor(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 MockLockFileSync({throwOnRemove: true});
+      const lockFile = new MockLockFile({throwOnRemove: true});
-      executor = new SingleProcessExecutorSync(
+      executor =
-          mockLogger, null as unknown as PackageJsonUpdater, lockFile);
+          new SingleProcessExecutor(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,10 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import {MockFileSystemNative} from '../../../src/ngtsc/file_system/testing';
-import {LockFileAsync, LockFileSync} from '../../src/execution/lock_file';
+import {LockFile} from '../../src/execution/lock_file';
-import {MockLogger} from './mock_logger';
 
-export class MockLockFileSync extends LockFileSync {
+export class MockLockFile extends LockFile {
   log: string[] = [];
   constructor(private options: {throwOnCreate?: boolean, throwOnRemove?: boolean} = {}) {
     // This `MockLockFile` is not used in tests that are run via `runInEachFileSystem()`
@@ -25,20 +24,3 @@ export class MockLockFileSync extends LockFileSync {
     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 {LockFileSync} from '../../src/execution/lock_file';
+import {LockFile} 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 LockFileSync(fs).lockFilePath));
+        fs.ensureDir(fs.dirname(new LockFile(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 call the delegate `readFile()` if the path for the cached file has been invalidated',
+    it('should use the delegate for `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 call the delegate `exists()` if the path for the cached file has been invalidated',
+    it('should use 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