Clean up child process exits on Windows #3817
Conversation
In Windows we are closing the process handle when the process successfully exits. However, the wait timer in `ProcessMonitor` repeatedly calls wait on the child, even after it's exited. This change makes `ProcessMonitor` and `_ProcessWindows` cache final values for the process exit status, so repeated calls to wait on the child will not be referencing an invalid handle. This also fixes a combinatorial explosion of timers in `ProcessMonitor._wait_for_child()`.
chalcolith
added
the
changelog - fixed
|
Hi @kulibali, The changelog - fixed label was added to this pull request; all PRs with a changelog label need to have release notes included as part of the PR. If you haven't added release notes already, please do. Release notes are added by creating a uniquely named file in the The basic format of the release notes (using markdown) should be: Thanks. |
| let hProcess: USize | ||
| let processError: (ProcessError | None) | ||
| var finalWaitResult: (_WaitResult | None) = None |
There was a problem hiding this comment.
per the style guide, we use snake case for fields.
I'll open an issue for that.
https://github.com/ponylang/ponyc/blob/main/STYLE_GUIDE.md#naming
|
@kulibali is there any way to test this in an automated fashion? if not, i feel like, as part of the commit comment for this, there should be a note for future reference on how to manually test. |
|
I'll take a look at adding a test. |
…xit status more than once after a child has exited
In Windows we are closing the process handle when the process successfully exits. However, the wait timer in
ProcessMonitorrepeatedly calls wait on the child, even after it's exited.This change makes
ProcessMonitorand_ProcessWindowscache final values for the process exit status, so repeated calls to wait on the child will not be referencing an invalid handle.This also fixes a combinatorial explosion of timers in
ProcessMonitor._wait_for_child().