diff --git a/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/index.ts b/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/index.ts new file mode 100644 index 0000000000..8cde3e462b --- /dev/null +++ b/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/index.ts @@ -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'; + +/// + +/** + * 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}); + } +} diff --git a/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/unlocker.ts b/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/unlocker.ts new file mode 100644 index 0000000000..798613541a --- /dev/null +++ b/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/unlocker.ts @@ -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'; + +/// + +// 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); }); diff --git a/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/util.ts b/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/util.ts new file mode 100644 index 0000000000..2ed0110d43 --- /dev/null +++ b/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/util.ts @@ -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; + } + } +} diff --git a/packages/compiler-cli/ngcc/src/main.ts b/packages/compiler-cli/ngcc/src/main.ts index 36c37ca83d..b58a021207 100644 --- a/packages/compiler-cli/ngcc/src/main.ts +++ b/packages/compiler-cli/ngcc/src/main.ts @@ -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); diff --git a/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/index_spec.ts b/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/index_spec.ts new file mode 100644 index 0000000000..ae7287b917 --- /dev/null +++ b/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/index_spec.ts @@ -0,0 +1,135 @@ +/** + * @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} from 'child_process'; +import * as process from 'process'; + +import {CachedFileSystem, FileSystem, getFileSystem} from '../../../../src/ngtsc/file_system'; +import {runInEachFileSystem} from '../../../../src/ngtsc/file_system/testing'; +import {getLockFilePath} from '../../../src/locking/lock_file'; +import {LockFileWithChildProcess} from '../../../src/locking/lock_file_with_child_process'; +import {MockLogger} from '../../helpers/mock_logger'; + +runInEachFileSystem(() => { + describe('LockFileWithChildProcess', () => { + /** + * This class allows us to test ordering of the calls, and to avoid actually attaching signal + * handlers and most importantly not actually exiting the process. + */ + class LockFileUnderTest extends LockFileWithChildProcess { + // Note that log is initialized in the `createUnlocker()` function that is called from + // super(), so we can't initialize it here. + log !: string[]; + constructor(fs: FileSystem) { + super(fs, new MockLogger()); + fs.ensureDir(fs.dirname(this.path)); + } + remove() { + this.log.push('remove()'); + super.remove(); + } + write() { + this.log.push('write()'); + super.write(); + } + read() { + const contents = super.read(); + this.log.push('read() => ' + contents); + return contents; + } + createUnlocker(): ChildProcess { + this.log = this.log || []; + this.log.push('createUnlocker()'); + const log = this.log; + // Normally this would fork a child process and return it. + // But we don't want to do that in these tests. + return {disconnect() { log.push('unlocker.disconnect()'); }}; + } + } + + describe('constructor', () => { + it('should create the unlocker process', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + expect(lockFile.log).toEqual(['createUnlocker()']); + }); + }); + + describe('write()', () => { + it('should write the lock-file to disk', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + expect(fs.exists(getLockFilePath(fs))).toBe(false); + lockFile.write(); + expect(fs.exists(getLockFilePath(fs))).toBe(true); + expect(fs.readFile(getLockFilePath(fs))).toEqual('' + process.pid); + }); + + it('should create the unlocker process if it is not already created', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + lockFile.log = []; + (lockFile as any).unlocker = null; + lockFile.write(); + expect(lockFile.log).toEqual(['write()', 'createUnlocker()']); + expect((lockFile as any).unlocker).not.toBe(null); + }); + }); + + describe('read()', () => { + it('should return the contents of the lock-file', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + fs.writeFile(lockFile.path, '' + process.pid); + expect(lockFile.read()).toEqual('' + process.pid); + }); + + it('should return `{unknown}` if the lock-file does not exist', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + expect(lockFile.read()).toEqual('{unknown}'); + }); + + it('should not read file from the cache, since the file may have been modified externally', + () => { + const rawFs = getFileSystem(); + const fs = new CachedFileSystem(rawFs); + const lockFile = new LockFileUnderTest(fs); + rawFs.writeFile(lockFile.path, '' + process.pid); + expect(lockFile.read()).toEqual('' + process.pid); + // We need to write to the rawFs to ensure that we don't update the cache at this point + rawFs.writeFile(lockFile.path, '444'); + expect(lockFile.read()).toEqual('444'); + }); + }); + + describe('remove()', () => { + it('should remove the lock file from the file-system', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + fs.writeFile(lockFile.path, '' + process.pid); + lockFile.remove(); + expect(fs.exists(lockFile.path)).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 disconnect the unlocker child process', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + fs.writeFile(lockFile.path, '' + process.pid); + lockFile.remove(); + expect(lockFile.log).toEqual(['createUnlocker()', 'remove()', 'unlocker.disconnect()']); + expect((lockFile as any).unlocker).toBe(null); + }); + }); + }); +}); diff --git a/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/unlocker_spec.ts b/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/unlocker_spec.ts new file mode 100644 index 0000000000..d6d5d697ce --- /dev/null +++ b/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/unlocker_spec.ts @@ -0,0 +1,17 @@ +/** + * @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 + */ + +/// + +describe('unlocker', () => { + it('should attach a handler to the `disconnect` event', () => { + spyOn(process, 'on'); + require('../../../src/locking/lock_file_with_child_process/unlocker'); + expect(process.on).toHaveBeenCalledWith('disconnect', jasmine.any(Function)); + }); +}); diff --git a/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/util_spec.ts b/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/util_spec.ts new file mode 100644 index 0000000000..b0ffe4c672 --- /dev/null +++ b/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/util_spec.ts @@ -0,0 +1,51 @@ +/** + * @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, absoluteFrom, getFileSystem} from '../../../../src/ngtsc/file_system'; +import {runInEachFileSystem} from '../../../../src/ngtsc/file_system/testing'; +import {removeLockFile} from '../../../src/locking/lock_file_with_child_process/util'; +import {MockLogger} from '../../helpers/mock_logger'; + +runInEachFileSystem(() => { + describe('LockFileWithChildProcess utils', () => { + let lockFilePath: AbsoluteFsPath; + let fs: FileSystem; + let logger: MockLogger; + + beforeEach(() => { + fs = getFileSystem(); + logger = new MockLogger(); + lockFilePath = absoluteFrom('/lockfile/path'); + fs.ensureDir(absoluteFrom('/lockfile')); + }); + + describe('removeLockFile()', () => { + it('should do nothing if there is no file to remove', + () => { removeLockFile(fs, logger, absoluteFrom('/lockfile/path'), '1234'); }); + + it('should do nothing if the pid does not match', () => { + fs.writeFile(lockFilePath, '888'); + removeLockFile(fs, logger, lockFilePath, '1234'); + expect(fs.exists(lockFilePath)).toBe(true); + expect(fs.readFile(lockFilePath)).toEqual('888'); + }); + + it('should remove the file if the pid matches', () => { + fs.writeFile(lockFilePath, '1234'); + removeLockFile(fs, logger, lockFilePath, '1234'); + expect(fs.exists(lockFilePath)).toBe(false); + }); + + it('should re-throw any other error', () => { + spyOn(fs, 'removeFile').and.throwError('removeFile() error'); + fs.writeFile(lockFilePath, '1234'); + expect(() => removeLockFile(fs, logger, lockFilePath, '1234')) + .toThrowError('removeFile() error'); + }); + }); + }); +});