refactor(ngcc): separate (Async/Sync)Locker
and LockFile
(#35861)
The previous implementation mixed up the management of locking a piece of code (both sync and async) with the management of writing and removing the lockFile that is used as the flag for which process has locked the code. This change splits these two concepts up. Apart from avoiding the awkward base class it allows the `LockFile` implementation to be replaced cleanly. PR Close #35861
This commit is contained in:

committed by
Matias Niemelä

parent
bdaab4184d
commit
94fa140888
@ -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 {AsyncLocker} 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: AsyncLocker) {}
|
||||
|
||||
async execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn):
|
||||
Promise<void> {
|
||||
|
@ -7,59 +7,80 @@
|
||||
*/
|
||||
import * as process from 'process';
|
||||
|
||||
import {CachedFileSystem, FileSystem} from '../../../src/ngtsc/file_system';
|
||||
import {AbsoluteFsPath, CachedFileSystem, FileSystem} from '../../../src/ngtsc/file_system';
|
||||
import {Logger} from '../logging/logger';
|
||||
|
||||
export abstract class LockFileBase {
|
||||
lockFilePath =
|
||||
this.fs.resolve(require.resolve('@angular/compiler-cli/ngcc'), '../__ngcc_lock_file__');
|
||||
let _lockFilePath: AbsoluteFsPath;
|
||||
export function getLockFilePath(fs: FileSystem) {
|
||||
if (!_lockFilePath) {
|
||||
_lockFilePath =
|
||||
fs.resolve(require.resolve('@angular/compiler-cli/ngcc'), '../__ngcc_lock_file__');
|
||||
}
|
||||
return _lockFilePath;
|
||||
}
|
||||
|
||||
export interface LockFile {
|
||||
path: AbsoluteFsPath;
|
||||
/**
|
||||
* Write a lock file to disk containing the PID of the current process.
|
||||
*/
|
||||
write(): void;
|
||||
|
||||
/**
|
||||
* Read the PID, of the process holding the lock, from the lockFile.
|
||||
*
|
||||
* It is feasible that the lockFile was removed between the call to `write()` that effectively
|
||||
* checks for existence and this attempt to read the file. If so then this method should just
|
||||
* gracefully return `"{unknown}"`.
|
||||
*/
|
||||
read(): string;
|
||||
|
||||
/**
|
||||
* Remove the lock file from disk, whether or not it exists.
|
||||
*/
|
||||
remove(): void;
|
||||
}
|
||||
|
||||
export class LockFileWithSignalHandlers implements LockFile {
|
||||
constructor(protected fs: FileSystem) {}
|
||||
|
||||
protected writeLockFile(): void {
|
||||
path = getLockFilePath(this.fs);
|
||||
|
||||
write(): void {
|
||||
try {
|
||||
this.addSignalHandlers();
|
||||
// To avoid race conditions, we check for existence of the lockfile
|
||||
// by actually trying to create it exclusively.
|
||||
return this.fs.writeFile(this.lockFilePath, process.pid.toString(), /* exclusive */ true);
|
||||
// To avoid race conditions, we check for existence of the lockFile by actually trying to
|
||||
// create it exclusively.
|
||||
return this.fs.writeFile(this.path, process.pid.toString(), /* exclusive */ true);
|
||||
} catch (e) {
|
||||
this.removeSignalHandlers();
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 readLockFile(): string {
|
||||
read(): string {
|
||||
try {
|
||||
if (this.fs instanceof CachedFileSystem) {
|
||||
// This file is "volatile", it might be changed by an external process,
|
||||
// so we cannot rely upon the cached value when reading it.
|
||||
this.fs.invalidateCaches(this.lockFilePath);
|
||||
this.fs.invalidateCaches(this.path);
|
||||
}
|
||||
return this.fs.readFile(this.lockFilePath);
|
||||
return this.fs.readFile(this.path);
|
||||
} catch {
|
||||
return '{unknown}';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove the lock file from disk.
|
||||
*/
|
||||
protected remove() {
|
||||
remove() {
|
||||
this.removeSignalHandlers();
|
||||
if (this.fs.exists(this.lockFilePath)) {
|
||||
this.fs.removeFile(this.lockFilePath);
|
||||
if (this.fs.exists(this.path)) {
|
||||
this.fs.removeFile(this.path);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Capture CTRL-C and terminal closing events.
|
||||
* When these occur we remove the lockfile and exit.
|
||||
* When these occur we remove the lockFile and exit.
|
||||
*/
|
||||
protected addSignalHandlers() {
|
||||
process.addListener('SIGINT', this.signalHandler);
|
||||
@ -94,14 +115,16 @@ export abstract class LockFileBase {
|
||||
}
|
||||
|
||||
/**
|
||||
* LockFileSync is used to prevent more than one instance of ngcc executing at the same time,
|
||||
* SyncLocker 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 {
|
||||
export class SyncLocker {
|
||||
constructor(private lockFile: LockFile) {}
|
||||
|
||||
/**
|
||||
* Run the given function guarded by the lock file.
|
||||
*
|
||||
@ -113,7 +136,7 @@ export class LockFileSync extends LockFileBase {
|
||||
try {
|
||||
return fn();
|
||||
} finally {
|
||||
this.remove();
|
||||
this.lockFile.remove();
|
||||
}
|
||||
}
|
||||
|
||||
@ -122,7 +145,7 @@ export class LockFileSync extends LockFileBase {
|
||||
*/
|
||||
protected create(): void {
|
||||
try {
|
||||
this.writeLockFile();
|
||||
this.lockFile.write();
|
||||
} catch (e) {
|
||||
if (e.code !== 'EEXIST') {
|
||||
throw e;
|
||||
@ -132,20 +155,20 @@ export class LockFileSync extends LockFileBase {
|
||||
}
|
||||
|
||||
/**
|
||||
* The lockfile already exists so raise a helpful error.
|
||||
* The lockFile already exists so raise a helpful error.
|
||||
*/
|
||||
protected handleExistingLockFile(): void {
|
||||
const pid = this.readLockFile();
|
||||
const pid = this.lockFile.read();
|
||||
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}.)`);
|
||||
`(If you are sure no ngcc process is running then you should delete the lockFile at ${this.lockFile.path}.)`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* LockFileAsync is used to prevent more than one instance of ngcc executing at the same time,
|
||||
* AsyncLocker 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.
|
||||
@ -155,12 +178,10 @@ export class LockFileSync extends LockFileBase {
|
||||
* * 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 {
|
||||
export class AsyncLocker {
|
||||
constructor(
|
||||
fs: FileSystem, protected logger: Logger, private retryDelay: number,
|
||||
private retryAttempts: number) {
|
||||
super(fs);
|
||||
}
|
||||
private lockFile: LockFile, protected logger: Logger, private retryDelay: number,
|
||||
private retryAttempts: number) {}
|
||||
|
||||
/**
|
||||
* Run a function guarded by the lock file.
|
||||
@ -169,19 +190,19 @@ export class LockFileAsync extends LockFileBase {
|
||||
*/
|
||||
async lock<T>(fn: () => Promise<T>): Promise<T> {
|
||||
await this.create();
|
||||
return fn().finally(() => this.remove());
|
||||
return fn().finally(() => this.lockFile.remove());
|
||||
}
|
||||
|
||||
protected async create() {
|
||||
let pid: string = '';
|
||||
for (let attempts = 0; attempts < this.retryAttempts; attempts++) {
|
||||
try {
|
||||
return this.writeLockFile();
|
||||
return this.lockFile.write();
|
||||
} catch (e) {
|
||||
if (e.code !== 'EEXIST') {
|
||||
throw e;
|
||||
}
|
||||
const newPid = this.readLockFile();
|
||||
const newPid = this.lockFile.read();
|
||||
if (newPid !== pid) {
|
||||
// The process locking the file has changed, so restart the timeout
|
||||
attempts = 0;
|
||||
@ -199,6 +220,6 @@ export class LockFileAsync extends LockFileBase {
|
||||
// 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}.)`);
|
||||
`(If you are sure no ngcc process is running then you should delete the lockFile at ${this.lockFile.path}.)`);
|
||||
}
|
||||
}
|
||||
|
@ -10,7 +10,7 @@ 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 {AsyncLocker, SyncLocker} from './lock_file';
|
||||
import {onTaskCompleted} from './utils';
|
||||
|
||||
export abstract class SingleProcessorExecutorBase {
|
||||
@ -43,11 +43,11 @@ export abstract class SingleProcessorExecutorBase {
|
||||
* 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) {
|
||||
constructor(logger: Logger, pkgJsonUpdater: PackageJsonUpdater, private lockFile: SyncLocker) {
|
||||
super(logger, pkgJsonUpdater);
|
||||
}
|
||||
execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn): void {
|
||||
this.lockfile.lock(() => this.doExecute(analyzeEntryPoints, createCompileFn));
|
||||
this.lockFile.lock(() => this.doExecute(analyzeEntryPoints, createCompileFn));
|
||||
}
|
||||
}
|
||||
|
||||
@ -55,11 +55,11 @@ export class SingleProcessExecutorSync extends SingleProcessorExecutorBase imple
|
||||
* An `Executor` that processes all tasks serially, but still completes asynchronously.
|
||||
*/
|
||||
export class SingleProcessExecutorAsync extends SingleProcessorExecutorBase implements Executor {
|
||||
constructor(logger: Logger, pkgJsonUpdater: PackageJsonUpdater, private lockfile: LockFileAsync) {
|
||||
constructor(logger: Logger, pkgJsonUpdater: PackageJsonUpdater, private lockFile: AsyncLocker) {
|
||||
super(logger, pkgJsonUpdater);
|
||||
}
|
||||
async execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn):
|
||||
Promise<void> {
|
||||
await this.lockfile.lock(async() => this.doExecute(analyzeEntryPoints, createCompileFn));
|
||||
await this.lockFile.lock(async() => this.doExecute(analyzeEntryPoints, createCompileFn));
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user