process icon indicating copy to clipboard operation
process copied to clipboard

WIP: make waitForProcess async safe

Open snoyberg opened this issue 3 years ago • 3 comments

Moving from https://github.com/fpco/typed-process/issues/38. CC @norfairking

This PR is not complete, the tests fail. I'm trying to determine whether this is a reasonable approach. My understanding of the issue here:

  1. waitForProcess wants to allow the C FFI call c_waitForProcess to be interruptible, so that waitpid can be interrupted
  2. If an async exception lands at exactly the wrong time, the waitpid call with succeed, but the FFI call will throw an exception due to the async exception
  3. In this case, the system process table no longer has an entry for the given PID, but the MVar inside ProcessHandle believes that the handle is still open, allowing future waitForProcess calls to occur
  4. Future calls throw an exception, since it's using an unknown PID

My best guess is that, in order to both allow waitpid to be interrupted and to reliably capture the exit code if it's generated, we need to:

  1. No longer rely on return codes from c_waitForProcess, since those will be lost in the case of an async exception
  2. Therefore, we need to detect whether the waitpid call succeeded or not by passing in another int* (along with the existing output parameter pret).
  3. Even if c_waitForProcess throws an exception, we need to update the MVar with the exit code, if it's returned successfully

I've taken a stab at this in this PR. @simonmar I was hoping you could have a look and let me know if this approach works, and/or if there's a better way to attack this kind of problem.

snoyberg avatar Mar 21 '21 08:03 snoyberg

@snoyberg Thanks for CC-ing me. I don't feel qualified to review this PR, to be honest. I'm betting that @nh2 will, if he has some spare cycles to help with this.

NorfairKing avatar Mar 22 '21 04:03 NorfairKing

I thought a bit about it and will try to reply to reply tomorrow.

nh2 avatar Mar 23 '21 04:03 nh2

@simonmar Gentle ping about this.

NorfairKing avatar Apr 20 '21 07:04 NorfairKing