Ensure that waitForProcess is never called more than once (fixes #69)

[sol]

This fixes the issue described in #69:

ghci> import Control.Exception
ghci> import System.Process.Typed
ghci> let config = setStdout createPipe (proc "echo" ["foo"])
ghci> withProcessWait config $ \ p -> Data.ByteString.hGetContents (getStdout p) >> throwIO DivideByZero
*** Exception: waitForProcess: does not exist (No child processes)

This is what I think is happening:

1. We cancel the waitingThread here:

   typed-process/src/System/Process/Typed.hs

   Line 247 in 788209f

   cancel waitingThread

2. The external echo process terminates normally and the waitingThread receives AsyncCancelled right after waitForProcess returns:

   typed-process/src/System/Process/Typed.hs

   Line 226 in 788209f

   then P.waitForProcess pHandle

   Note that at this point we lost the ExitCode: waitForProcess has concluded, but pExitCode is still empty.

3. typed-process/src/System/Process/Typed.hs

   Line 250 in 788209f

   eec <- waitCatch waitingThread

   Now waitCatch returns a Left-value here and we erroneously assume that waitForProcess did not conclude yet.
4. We call waitForProcess a second time which results in an IOException of type NoSuchThing:

   typed-process/src/System/Process/Typed.hs

   Line 277 in 788209f

   Right () -> P.waitForProcess pHandle

This PR always relies on the waitingThread to call waitForProcess and by extension structures the code in a away so that waitForProcess can only ever be called once.

[sol]

@snoyberg this is ready to go. I edited the PR description to include the exact execution sequence that I think led to the issue.

[sol]

Also note that this will allow us to further simplify the code if we choose to, to make sure that we can't possibly forget to put the ExitCode into the TMVar (sol@3e37a48).

[snoyberg]

Overall this looks OK to me, but to be honest this has been a very sensitive part of the code and I don't really remember all the different edge cases. @nh2 would you mind giving this a look over as well?

[sol]

Thanks @snoyberg for taking a look.

I don't want to rush this in, but I have a (private) PR pending, that is blocked by this bug fix. So, naturally, I care about getting this merged.

Is there anything I can do to help move this forward? Would it help if I add more verbose comments on how I think best to review this?

[snoyberg]

I've pinged some people for review, but I'm not comfortable moving ahead on my own review alone, I know this code has gone through lots of revisions with lots of different bugs coming from each iteration.

[sol]

Thanks @snoyberg. Can we get #71 merged first? I would then rebase this PR. This will give us a much smaller diff, which I hope will significantly aid in reviewing it.

Note that #71 does not change any behavior.

[snoyberg]

I see that @tomjaguarpaw already commented on that PR, so I'd defer to him. Personally, I find the change in code harder to parse than the original version.

[sol]

Most of the time this will be a Right-value, but with delegate_ctlc this can be UserInterrupt, which we then re-throw here (see #73).