Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generate nonzero exit code when yarn 2 process is killed #8793

Closed
wants to merge 23 commits into from

Conversation

clhuang
Copy link

@clhuang clhuang commented Mar 4, 2022

Summary

Yarn 2+, when SIGKILLed (due to OOM for example), fails silently as the yarn 1 process that spawns it does not recognize that it was killed (see yarnpkg/berry#3996), resulting in issues that are difficult to debug as it appears to be successful.

Thus, in spawnp and forkp, use the bash convention of 128+signal to notify the yarn caller that the yarn 2 process was terminated via signal.

Test plan

@Harry-Hopkinson
Copy link

Create a pull request in the new development trunk.

@clhuang
Copy link
Author

clhuang commented Mar 21, 2022

@Harry-Hopkinson Are you saying I should make a PR in https://github.com/yarnpkg/berry? This bug cannot be fixed in the yarn 2 codebase; when yarn 2 is SIGKILLed it is the yarn 1 code that ignores the signal and returns the exit code 0. It is impossible for yarn 2 to handle the SIGKILL.

@Harry-Hopkinson
Copy link

Ah okay - possibly contact one of the main contributors to the repository due to this development trunk being archived.

@clhuang
Copy link
Author

clhuang commented Mar 21, 2022

@arcanis this fixes yarnpkg/berry#3996

@mhassan1
Copy link

@clhuang @arcanis @merceyz Are there any blockers to this PR?

@arcanis arcanis changed the base branch from master to 1.22-stable November 10, 2023 16:36
@merceyz
Copy link
Member

merceyz commented Nov 14, 2023

Fixed in #9009

@merceyz merceyz closed this Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants