diff --git a/packages/compiler-cli/ngcc/src/locking/async_locker.ts b/packages/compiler-cli/ngcc/src/locking/async_locker.ts index f0ea1cebad..7e2f1dc7aa 100644 --- a/packages/compiler-cli/ngcc/src/locking/async_locker.ts +++ b/packages/compiler-cli/ngcc/src/locking/async_locker.ts @@ -60,68 +60,22 @@ export class AsyncLocker { pid = newPid; } if (attempts === 0) { - // Check to see if the process identified by the PID is still running. Because the - // process *should* clean up after itself, we only check for a stale lock file when the - // PID changes and only once. This may mean you have to wait if the process is killed - // after the first check and isn't given the chance to clean up after itself. - if (!this.isProcessRunning(pid)) { - // try to re-lock one last time in case there was a race condition checking the process. - try { - return this.lockFile.write(); - } catch (e2) { - if (e2.code !== 'EEXIST') { - throw e2; - } - } - - // finally check that the lock was held by the same process this whole time. - const finalPid = this.lockFile.read(); - if (finalPid === pid) { - throw new TimeoutError(this.lockFileMessage( - `Lock found, but no process with PID ${pid} seems to be running.`)); - } else { - // attempts is still 0, but adjust the PID so the message below is correct. - pid = finalPid; - } - } - - this.logger.info(this.lockFileMessage( + 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.`)); + `Waiting up to ${this.retryDelay * this.retryAttempts / 1000}s for it to finish.\n` + + `(If you are sure no ngcc process is running then you should delete the lock-file at ${ + this.lockFile.path}.)`); } // 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 TimeoutError(this.lockFileMessage(`Timed out waiting ${ - this.retryAttempts * this.retryDelay / - 1000}s for another ngcc process, with id ${pid}, to complete.`)); - } - - protected isProcessRunning(pid: string): boolean { - // let the normal logic run if this is not called with a valid PID - if (isNaN(+pid)) { - this.logger.debug(`Cannot check if invalid PID "${pid}" is running, a number is expected.`); - return true; - } - - try { - process.kill(+pid, 0); - return true; - } catch (e) { - // If the process doesn't exist ESRCH will be thrown, if the error is not that, throw it. - if (e.code !== 'ESRCH') { - throw e; - } - - return false; - } - } - - private lockFileMessage(message: string): string { - return message + - `\n(If you are sure no ngcc process is running then you should delete the lock-file at ${ - this.lockFile.path}.)`; + throw new TimeoutError( + `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 lock-file at ${ + this.lockFile.path}.)`); } } diff --git a/packages/compiler-cli/ngcc/test/locking/async_locker_spec.ts b/packages/compiler-cli/ngcc/test/locking/async_locker_spec.ts index 3cd2374ecb..cfee2cfd01 100644 --- a/packages/compiler-cli/ngcc/test/locking/async_locker_spec.ts +++ b/packages/compiler-cli/ngcc/test/locking/async_locker_spec.ts @@ -71,7 +71,6 @@ runInEachFileSystem(() => { } return lockFileContents; }); - spyOn(process, 'kill').and.returnValue(); const promise = locker.lock(async () => log.push('fn()')); // The lock is now waiting on the lock-file becoming free, so no `fn()` in the log. @@ -81,7 +80,6 @@ runInEachFileSystem(() => { `(If you are sure no ngcc process is running then you should delete the lock-file at ${ lockFile.path}.)` ]]); - expect(process.kill).toHaveBeenCalledWith(188, 0); lockFileContents = null; // The lock-file has been removed, so we can create our own lock-file, call `fn()` and then @@ -90,47 +88,6 @@ runInEachFileSystem(() => { expect(log).toEqual(['write()', 'read() => 188', 'write()', 'fn()', 'remove()']); }); - it('should fail fast when waiting on a dead process', async () => { - const fs = getFileSystem(); - const log: string[] = []; - const lockFile = new MockLockFile(fs, log); - const logger = new MockLogger(); - const locker = new AsyncLocker(lockFile, logger, 100, 10); - - let lockFileContents: string|null = '188'; - spyOn(lockFile, 'write').and.callFake(() => { - log.push('write()'); - if (lockFileContents) { - throw {code: 'EEXIST'}; - } - }); - spyOn(lockFile, 'read').and.callFake(() => { - log.push('read() => ' + lockFileContents); - if (lockFileContents === null) { - throw {code: 'ENOENT'}; - } - return lockFileContents; - }); - spyOn(process, 'kill').and.callFake(() => { - throw {code: 'ESRCH'}; - }); - - const promise = locker.lock(async () => log.push('fn()')); - // The lock has already failed so no `fn()` in the log. - expect(log).toEqual(['write()', 'read() => 188', 'write()', 'read() => 188']); - expect(logger.logs.info).toEqual([]); - expect(process.kill).toHaveBeenCalledWith(188, 0); - // Check that a missing process errors out. - let error: Error; - await promise.catch(e => error = e); - expect(log).toEqual(['write()', 'read() => 188', 'write()', 'read() => 188']); - expect(error!.message) - .toEqual( - `Lock found, but no process with PID 188 seems to be running.\n` + - `(If you are sure no ngcc process is running then you should delete the lock-file at ${ - lockFile.path}.)`); - }); - it('should extend the retry timeout if the other process locking the file changes', async () => { const fs = getFileSystem(); const log: string[] = []; @@ -152,7 +109,6 @@ runInEachFileSystem(() => { } return lockFileContents; }); - spyOn(process, 'kill').and.returnValue(); const promise = locker.lock(async () => log.push('fn()')); // The lock is now waiting on the lock-file becoming free, so no `fn()` in the log. @@ -162,7 +118,6 @@ runInEachFileSystem(() => { `(If you are sure no ngcc process is running then you should delete the lock-file at ${ lockFile.path}.)` ]]); - expect(process.kill).toHaveBeenCalledWith(188, 0); lockFileContents = '444'; // The lock-file has been taken over by another process - wait for the next attempt @@ -176,7 +131,6 @@ runInEachFileSystem(() => { `(If you are sure no ngcc process is running then you should delete the lock-file at ${ lockFile.path}.)`] ]); - expect(process.kill).toHaveBeenCalledWith(444, 0); lockFileContents = null; // The lock-file has been removed, so we can create our own lock-file, call `fn()` and @@ -209,13 +163,11 @@ runInEachFileSystem(() => { } return lockFileContents; }); - spyOn(process, 'kill').and.returnValue(); const promise = locker.lock(async () => log.push('fn()')); // The lock is now waiting on the lock-file becoming free, so no `fn()` in the log. expect(log).toEqual(['write()', 'read() => 188']); - expect(process.kill).toHaveBeenCalledWith(188, 0); // Do not remove the lock-file and let the call to `lock()` timeout. let error: Error; await promise.catch(e => error = e);