fix(ngcc): a new LockFile implementation that uses a child-process (#35861)
This version of `LockFile` creates an "unlocker" child-process that monitors the main ngcc process and deletes the lock file if it exits unexpectedly. This resolves the issue where the main process could not be killed by pressing Ctrl-C at the terminal. Fixes #35761 PR Close #35861
This commit is contained in:

committed by
Matias Niemelä

parent
4acd658635
commit
c55f900081
@ -0,0 +1,86 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright Google Inc. All Rights Reserved.
|
||||
*
|
||||
* Use of this source code is governed by an MIT-style license that can be
|
||||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
import {ChildProcess, fork} from 'child_process';
|
||||
|
||||
import {AbsoluteFsPath, CachedFileSystem, FileSystem} from '../../../../src/ngtsc/file_system';
|
||||
import {LogLevel, Logger} from '../../logging/logger';
|
||||
import {LockFile, getLockFilePath} from '../lock_file';
|
||||
|
||||
import {removeLockFile} from './util';
|
||||
|
||||
/// <reference types="node" />
|
||||
|
||||
/**
|
||||
* This `LockFile` implementation uses a child-process to remove the lock file when the main process
|
||||
* exits (for whatever reason).
|
||||
*
|
||||
* There are a few milliseconds between the child-process being forked and it registering its
|
||||
* `disconnect` event, which is responsible for tidying up the lock-file in the event that the main
|
||||
* process exits unexpectedly.
|
||||
*
|
||||
* We eagerly create the unlocker child-process so that it maximizes the time before the lock-file
|
||||
* is actually written, which makes it very unlikely that the unlocker would not be ready in the
|
||||
* case that the developer hits Ctrl-C or closes the terminal within a fraction of a second of the
|
||||
* lock-file being created.
|
||||
*
|
||||
* The worst case scenario is that ngcc is killed too quickly and leaves behind an orphaned
|
||||
* lock-file. In which case the next ngcc run will display a helpful error message about deleting
|
||||
* the lock-file.
|
||||
*/
|
||||
export class LockFileWithChildProcess implements LockFile {
|
||||
path: AbsoluteFsPath;
|
||||
private unlocker: ChildProcess|null;
|
||||
|
||||
constructor(protected fs: FileSystem, protected logger: Logger) {
|
||||
this.path = getLockFilePath(fs);
|
||||
this.unlocker = this.createUnlocker(this.path);
|
||||
}
|
||||
|
||||
|
||||
write(): void {
|
||||
if (this.unlocker === null) {
|
||||
// In case we already disconnected the previous unlocker child-process, perhaps by calling
|
||||
// `remove()`. Normally the LockFile should only be used once per instance.
|
||||
this.unlocker = this.createUnlocker(this.path);
|
||||
}
|
||||
this.logger.debug(`Attemping to write lock-file at ${this.path} with PID ${process.pid}`);
|
||||
// To avoid race conditions, check for existence of the lock-file by trying to create it.
|
||||
// This will throw an error if the file already exists.
|
||||
this.fs.writeFile(this.path, process.pid.toString(), /* exclusive */ true);
|
||||
this.logger.debug(`Written lock-file at ${this.path} with PID ${process.pid}`);
|
||||
}
|
||||
|
||||
read(): string {
|
||||
try {
|
||||
if (this.fs instanceof CachedFileSystem) {
|
||||
// The lock-file file is "volatile", it might be changed by an external process,
|
||||
// so we must not rely upon the cached value when reading it.
|
||||
this.fs.invalidateCaches(this.path);
|
||||
}
|
||||
return this.fs.readFile(this.path);
|
||||
} catch {
|
||||
return '{unknown}';
|
||||
}
|
||||
}
|
||||
|
||||
remove() {
|
||||
removeLockFile(this.fs, this.logger, this.path, process.pid.toString());
|
||||
if (this.unlocker !== null) {
|
||||
// If there is an unlocker child-process then disconnect from it so that it can exit itself.
|
||||
this.unlocker.disconnect();
|
||||
this.unlocker = null;
|
||||
}
|
||||
}
|
||||
|
||||
protected createUnlocker(path: AbsoluteFsPath): ChildProcess {
|
||||
this.logger.debug('Forking unlocker child-process');
|
||||
const logLevel =
|
||||
this.logger.level !== undefined ? this.logger.level.toString() : LogLevel.info.toString();
|
||||
return fork(this.fs.resolve(__dirname, './unlocker.js'), [path, logLevel], {detached: true});
|
||||
}
|
||||
}
|
@ -0,0 +1,39 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright Google Inc. All Rights Reserved.
|
||||
*
|
||||
* Use of this source code is governed by an MIT-style license that can be
|
||||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
import {NodeJSFileSystem} from '../../../../src/ngtsc/file_system';
|
||||
import {ConsoleLogger} from '../../logging/console_logger';
|
||||
import {removeLockFile} from './util';
|
||||
|
||||
/// <reference types="node" />
|
||||
|
||||
// This file is an entry-point for the child-process that is started by `LockFileWithChildProcess`
|
||||
// to ensure that the lock-file is removed when the primary process exits unexpectedly.
|
||||
|
||||
// We have no choice but to use the node.js file-system here since we are in a separate process
|
||||
// from the main ngcc run, which may be running a mock file-system from within a test.
|
||||
const fs = new NodeJSFileSystem();
|
||||
|
||||
// We create a logger that has the same logging level as the parent process, since it should have
|
||||
// been passed through as one of the args
|
||||
const logLevel = parseInt(process.argv.pop() !, 10);
|
||||
const logger = new ConsoleLogger(logLevel);
|
||||
|
||||
// We must store the parent PID now as it changes if the parent process is killed early
|
||||
const ppid = process.ppid.toString();
|
||||
|
||||
// The path to the lock-file to remove should have been passed as one of the args
|
||||
const lockFilePath = fs.resolve(process.argv.pop() !);
|
||||
|
||||
logger.debug(`Starting unlocker at process ${process.pid} on behalf of process ${ppid}`);
|
||||
logger.debug(`The lock-file path is ${lockFilePath}`);
|
||||
|
||||
/**
|
||||
* When the parent process exits (for whatever reason) remove the loc-file if it exists and as long
|
||||
* as it was one that was created by the parent process.
|
||||
*/
|
||||
process.on('disconnect', () => { removeLockFile(fs, logger, lockFilePath, ppid); });
|
@ -0,0 +1,37 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright Google Inc. All Rights Reserved.
|
||||
*
|
||||
* Use of this source code is governed by an MIT-style license that can be
|
||||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
import {AbsoluteFsPath, FileSystem} from '../../../../src/ngtsc/file_system';
|
||||
import {Logger} from '../../logging/logger';
|
||||
|
||||
/**
|
||||
* Remove the lock-file at the provided `lockFilePath` from the given file-system.
|
||||
*
|
||||
* It only removes the file if the pid stored in the file matches the provided `pid`.
|
||||
* The provided `pid` is of the process that is exiting and so no longer needs to hold the lock.
|
||||
*/
|
||||
export function removeLockFile(
|
||||
fs: FileSystem, logger: Logger, lockFilePath: AbsoluteFsPath, pid: string) {
|
||||
try {
|
||||
logger.debug(`Attempting to remove lock-file at ${lockFilePath}.`);
|
||||
const lockFilePid = fs.readFile(lockFilePath);
|
||||
if (lockFilePid === pid) {
|
||||
logger.debug(`PIDs match (${pid}), so removing ${lockFilePath}.`);
|
||||
fs.removeFile(lockFilePath);
|
||||
} else {
|
||||
logger.debug(
|
||||
`PIDs do not match (${pid} and ${lockFilePid}), so not removing ${lockFilePath}.`);
|
||||
}
|
||||
} catch (e) {
|
||||
if (e.code === 'ENOENT') {
|
||||
logger.debug(`The lock-file at ${lockFilePath} was already removed.`);
|
||||
// File already removed so quietly exit
|
||||
} else {
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
}
|
@ -31,7 +31,7 @@ import {SingleProcessExecutorAsync, SingleProcessExecutorSync} from './execution
|
||||
import {ParallelTaskQueue} from './execution/task_selection/parallel_task_queue';
|
||||
import {SerialTaskQueue} from './execution/task_selection/serial_task_queue';
|
||||
import {AsyncLocker} from './locking/async_locker';
|
||||
import {LockFileWithSignalHandlers} from './locking/lock_file_with_signal_handlers';
|
||||
import {LockFileWithChildProcess} from './locking/lock_file_with_child_process';
|
||||
import {SyncLocker} from './locking/sync_locker';
|
||||
import {ConsoleLogger} from './logging/console_logger';
|
||||
import {LogLevel, Logger} from './logging/logger';
|
||||
@ -338,7 +338,7 @@ function getTaskQueue(
|
||||
function getExecutor(
|
||||
async: boolean, inParallel: boolean, logger: Logger, pkgJsonUpdater: PackageJsonUpdater,
|
||||
fileSystem: FileSystem): Executor {
|
||||
const lockFile = new LockFileWithSignalHandlers(fileSystem);
|
||||
const lockFile = new LockFileWithChildProcess(fileSystem, logger);
|
||||
if (async) {
|
||||
// Execute asynchronously (either serially or in parallel)
|
||||
const locker = new AsyncLocker(lockFile, logger, 500, 50);
|
||||
|
Reference in New Issue
Block a user