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

SIGCHLD side effects #10

Closed
jeroen opened this issue Mar 12, 2017 · 6 comments
Closed

SIGCHLD side effects #10

jeroen opened this issue Mar 12, 2017 · 6 comments

Comments

@jeroen
Copy link
Owner

jeroen commented Mar 12, 2017

Our SIGCHLDhandler cleans child processes before the parent can get to the exit code. This affects all subprocesses:

res <- system("ping sdffsdsfsdfasdf.xyz", T)
attr(res, "status")
# [1] 68

However after sys is loaded the exit code is always 0:

library(sys)
res <- system("ping sdffsdsfsdfasdf.xyz", T)
attr(res, "status")
# NULL

This is bad. We should find a way to mark a child spawned by sys and only clean up these processes in our SIGCHLD handler.

@jeroen
Copy link
Owner Author

jeroen commented Mar 12, 2017

Alternatively we should abandon SIGCHLD and make the user manually clean children, or do it in some handle finalizer.

@gaborcsardi
Copy link
Contributor

Yeah, that's bad.

Alternatively we should abandon SIGCHLD and make the user manually clean children, or do it in some handle finalizer.

That's fine with me, I can write a wrapper.

@jeroen
Copy link
Owner Author

jeroen commented Mar 12, 2017

Actuall are you sure zombies are a problem in the first place? At least on OSX:

for(i in 1:100) print(exec_background('whoami'))

I am not seeing zombies in top, even when disabling the signal handler.

@jeroen
Copy link
Owner Author

jeroen commented Mar 12, 2017

How does the parallel SIGCHLD handler avoid this problem?

@jeroen
Copy link
Owner Author

jeroen commented Mar 12, 2017

Ah I see it now. mcparallel keep track of a list of children that were spawned and only cleans up those.

@jeroen
Copy link
Owner Author

jeroen commented Mar 12, 2017

I have added a exec_status function to check the exit status of a pid, and optionally wait for it. The SIGCHLD handler is now commented-out in master until we find a solution without side effects.

@gaborcsardi can you check if exec_background() + exec_status() suffices for your use cases?

I still need to implement this for windows btw.

@jeroen jeroen closed this as completed Nov 5, 2018
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

No branches or pull requests

2 participants