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

Processes started on Windows with use_process_jobs=True may be terminated #167

Closed
bgamari opened this issue Feb 3, 2020 · 3 comments
Closed

Comments

@bgamari
Copy link
Contributor

bgamari commented Feb 3, 2020

The current implementation of the use_process_jobs codepath on Windows (introduced in #80) has a few issues, the most significant of which can result in the process being unexpectedly terminated. Specifically, this issue can occur as follows:

  1. the user spawns a process with use_process_jobs=True
  2. the user waits on the ProcessHandle with waitForProcess but holds no other reference to the ProcessHandle
  3. waitForProcess safe-foreign-calls into waitForJobCompletion
  4. waitForJobCompletion blocks with GetQueuedCompletionStatus on the IOCP associated with the job
  5. a major GC occurs, the ProcessHandle is collected since there are no references to it
  6. the finalizer attached to the ProcessHandle's MVar is run, closing the process handles
  7. waitForJobCompletion is unblocked by some event, handles it, and tries blocking again
  8. GetQueuedCompletionStatus fails with an "invalid handle" error since the handle has been closed
  9. waitForJobCompletion sees the failure, breaks out of the loop, and returns with status code 0.
  10. the caller thinks that the process finished successfully and is later surprised when this isn't the case

The reason this happens is that the use_process_jobs codepath, unlike the other codepaths of waitForProcess, makes no attempt to do anything with the process handle (e.g. updating the MVar to be a ClosedHandle) after it finishes waiting.

In addition, the use_process_jobs codepath fails to call endDelegateControlC on process completion, unlike the "normal" codepath.

As an orthogonal matter, the documentation of use_process_jobs could be better; currently we don't adequately explain when a user should be using a job. In addition, we should note that exit codes may be incorrect on Windows when exec is used.

@bgamari
Copy link
Contributor Author

bgamari commented Feb 3, 2020

I have fixed this in https://github.com/bgamari/process/tree/fix-jobs, which I'm still testing.

@bgamari
Copy link
Contributor Author

bgamari commented Feb 3, 2020

Note: I just realized that endDelegateControlC is a no-op on Windows so this particular point in the ticket summary isn't relevant.

@bgamari bgamari mentioned this issue Feb 3, 2020
@bgamari
Copy link
Contributor Author

bgamari commented Feb 6, 2020

This was fixed in #168.

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

1 participant