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

tests: Wait for forked child processes #53

Merged
merged 1 commit into from Mar 20, 2016

Conversation

@antrik
Copy link
Contributor

antrik commented Mar 19, 2016

Don't leave zombie processes around -- they hinder debugging, and might
have other harmful side effects as well.

Don't leave zombie processes around -- they hinder debugging, and might
have other harmful side effects as well.
@antrik
Copy link
Contributor Author

antrik commented Mar 19, 2016

Refactored variant. Let's hope I didn't break Windows build...

@pcwalton
Copy link
Collaborator

pcwalton commented Mar 19, 2016

I don't think it's unsafe, but it doesn't really matter.

@pcwalton
Copy link
Collaborator

pcwalton commented Mar 19, 2016

@KiChjang
Copy link
Member

KiChjang commented Mar 20, 2016

@bors-servo r=pcwalton

@KiChjang KiChjang closed this Mar 20, 2016
@KiChjang KiChjang reopened this Mar 20, 2016
@KiChjang
Copy link
Member

KiChjang commented Mar 20, 2016

@bors-servo r=pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Mar 20, 2016

📌 Commit 027143f has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Mar 20, 2016

Testing commit 027143f with merge 51df604...

bors-servo added a commit that referenced this pull request Mar 20, 2016
tests: Wait for forked child processes

Don't leave zombie processes around -- they hinder debugging, and might
have other harmful side effects as well.
@bors-servo
Copy link
Contributor

bors-servo commented Mar 20, 2016

☀️ Test successful - travis

@bors-servo bors-servo merged commit 027143f into servo:master Mar 20, 2016
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
///XXXjdm Windows' libc doesn't include fork.
#[cfg(not(windows))]
// I'm not actually sure invoking this is indeed unsafe -- but better safe than sorry...
pub unsafe fn fork<F: FnOnce()>(child_func: F) -> libc::pid_t {

This comment has been minimized.

@nox

nox Mar 20, 2016

Member

I think F should be FnOnce() -> !, to show that it shouldn't return ever.

This comment has been minimized.

@antrik

antrik Mar 20, 2016

Author Contributor

For the record: I actually think so too; but because of language limitations, this can currently be only achieved by using the Void crate -- which @pcwalton didn't want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.