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

chore(scm): avoid dropping child before wait #9564

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

chris-olszewski
Copy link
Member

Description

In the case of an error when parsing git output. We would drop a Child without waiting on it which results in a zombie process as the pid is never reaped.

From Rust docs

On some systems, calling wait or similar is necessary for the OS to release resources. A process that terminated but has not been waited on is still around as a “zombie”. Leaving too many zombies around may exhaust global resources (for example process IDs).

The standard library does not automatically wait on child processes (not even if the Child is dropped), it is up to the application developer to do so. As a consequence, dropping Child handles without waiting on them first is not recommended in long-running applications.

When there was a parse error we would kill the child process, but never reap the pid. This PR ensures we make a best effort to do just that. The way I'm calling wait is probably overkill, but I wanted to ensure that we don't introduce any accidental waiting on a process that didn't receive the kill signal.

Sources for comments:

Testing Instructions

I have done some manual confirmation that this works for a command like bash -c "sleep 100" where it will

Hoping to get someone from #9455 to test this out in a canary and confirm this helps.

Copy link

vercel bot commented Dec 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 5, 2024 0:36am
8 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Dec 5, 2024 0:36am
examples-designsystem-docs ⬜️ Ignored (Inspect) Dec 5, 2024 0:36am
examples-gatsby-web ⬜️ Ignored (Inspect) Dec 5, 2024 0:36am
examples-kitchensink-blog ⬜️ Ignored (Inspect) Dec 5, 2024 0:36am
examples-native-web ⬜️ Ignored (Inspect) Dec 5, 2024 0:36am
examples-svelte-web ⬜️ Ignored (Inspect) Dec 5, 2024 0:36am
examples-tailwind-web ⬜️ Ignored (Inspect) Dec 5, 2024 0:36am
examples-vite-web ⬜️ Ignored (Inspect) Dec 5, 2024 0:36am

@chris-olszewski chris-olszewski marked this pull request as ready for review December 5, 2024 00:36
@chris-olszewski chris-olszewski requested a review from a team as a code owner December 5, 2024 00:36
@anthonyshew anthonyshew changed the title chore(scm): avoid droping child before wait chore(scm): avoid dropping child before wait Dec 5, 2024
@chris-olszewski chris-olszewski merged commit eef7137 into main Dec 5, 2024
37 checks passed
@chris-olszewski chris-olszewski deleted the olszewski/fix_git_child_drop branch December 5, 2024 13:38
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.

2 participants