Skip to content

Commit

Permalink
Don’t call release with an exit code (#6981)
Browse files Browse the repository at this point in the history
* Don’t call `release` with an exit code

The `release` signature is incorrect, because its first argument is a callback function that will be called when the release is done. By attaching it to the `run()` promise however, release will be called with the exit code as argument. When the exit code is `1`, the code will crash. 

This is very rare, but we’re seeing this when calling yarn from another node process (and possibly killing it too soon):
```
<redacted>/node_modules/yarn/lib/cli.js:78119
         callback();
         ^
 TypeError: callback is not a function
     at options.fs.rmdir (<redacted>/node_modules/yarn/lib/cli.js:78119:9)
     at FSReqWrap.oncomplete (fs.js:141:20)
```

* Only resolve after releasing is done, improve type of `release`.

Note that this slightly violates the type of runEventuallyWithFile, as
the Promised resolved by the release callback might return an Error
object. But since this error was also not handled before, it’s out of
scope for the fix of this patch.
  • Loading branch information
eelco authored and arcanis committed Feb 2, 2019
1 parent 65e198d commit 19ec98c
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/cli/index.js
Expand Up @@ -291,7 +291,7 @@ export async function main({
const runEventuallyWithFile = (mutexFilename: ?string, isFirstTime?: boolean): Promise<void> => {
return new Promise(resolve => {
const lockFilename = mutexFilename || path.join(config.cwd, constants.SINGLE_INSTANCE_FILENAME);
lockfile.lock(lockFilename, {realpath: false}, (err: mixed, release: () => void) => {
lockfile.lock(lockFilename, {realpath: false}, (err: mixed, release: (() => void) => void) => {
if (err) {
if (isFirstTime) {
reporter.warn(reporter.lang('waitingInstance'));
Expand All @@ -303,7 +303,7 @@ export async function main({
onDeath(() => {
process.exitCode = 1;
});
resolve(run().then(release));
resolve(run().then(() => new Promise(resolve => release(resolve))));
}
});
});
Expand Down

0 comments on commit 19ec98c

Please sign in to comment.