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

Wait on child PID after failed exec #14

Merged
merged 1 commit into from
Jan 11, 2015
Merged

Wait on child PID after failed exec #14

merged 1 commit into from
Jan 11, 2015

Conversation

DaveCTurner
Copy link
Contributor

On Linux, the following program generates 10 zombie processes, one for each time that callProcess tries to call a nonexistent process.

import Control.Concurrent
import Control.Exception
import Control.Monad
import System.IO
import System.Process

ignoreIOException :: IOException -> IO ()
ignoreIOException _ = return ()

main = do
  _ <- forkIO $ replicateM_ 10 $ callProcess "does-not-exist" [] `catch` ignoreIOException
  _ <- getLine
  return ()

The reason seems to be that the child PID leaks because the parent does not call waitpid() if the child has failed. This simple change seems to fix it.

@dcoutts
Copy link
Contributor

dcoutts commented Jan 10, 2015

After looking at it for a while, I agree.

Minor point, as far as I can see (pid > 0) is necessarily true at that point in the code, and I'd add a comment, e.g.

     // We forked the child, but the child had a problem and stopped
     // so it's our responsibility to reap here as nobody else can.
     waitpid(pid, &unused, 0);

@DaveCTurner
Copy link
Contributor Author

Seems reasonable. Do you want to make that change yourself or do you want another PR?

@DaveCTurner
Copy link
Contributor Author

Ok, 77dc6dc takes your suggestions into account.

The docs for waitpid() say "If status is not NULL,..." which sort of implies that NULL is acceptable when you don't care about the status. So I've used that.

dcoutts added a commit that referenced this pull request Jan 11, 2015
Wait on child PID after failed exec
@dcoutts dcoutts merged commit 29f0480 into haskell:master Jan 11, 2015
@dcoutts
Copy link
Contributor

dcoutts commented Jan 11, 2015

Thanks!

hvr added a commit that referenced this pull request Jan 15, 2015
This adds changelog entries for the issues #14 and #15 addressed in
1.2.2 so far.
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