Make ProcessWaitState store ProcessExitStatus instead of int? exit code#127010
Make ProcessWaitState store ProcessExitStatus instead of int? exit code#127010
Conversation
- Change _exitCode (int?) to _exitStatus (ProcessExitStatus?) in ProcessWaitState.Unix.cs - Update GetExited to return ProcessExitStatus? instead of int? - Extend Interop.Sys.WaitPidExitedNoHang with out int terminatingSignal parameter - Update SystemNative_WaitPidExitedNoHang native implementation to extract terminating signal via WTERMSIG and map it using TryConvertSignalCodeToPosixSignal - Make TryConvertSignalCodeToPosixSignal non-static in pal_signal.c and declare it in pal_signal.h - Update pal_process.h, pal_process_wasi.c signatures accordingly - Update ChildReaped and all callers to pass signal information through Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/3692424a-4bc2-4eca-84fe-0eb8b7981738 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
…posixSignal Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/3692424a-4bc2-4eca-84fe-0eb8b7981738 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/area-system-diagnostics-process |
There was a problem hiding this comment.
Pull request overview
Updates Unix process reaping plumbing so ProcessWaitState can cache a richer ProcessExitStatus (including terminating signal info) rather than only an int? exit code, as groundwork for upcoming SafeProcessHandle.WaitForExit* APIs.
Changes:
- Extend
SystemNative_WaitPidExitedNoHang/Interop.Sys.WaitPidExitedNoHangto return an additional “terminating signal” out parameter. - Plumb signal information through
ProcessWaitState.UnixviaProcessExitStatus?caching. - Make
TryConvertSignalCodeToPosixSignalcallable frompal_process.cby moving it to a header-visible helper.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/libs/System.Native/pal_signal.h | Declares TryConvertSignalCodeToPosixSignal for cross-file use. |
| src/native/libs/System.Native/pal_signal.c | Makes TryConvertSignalCodeToPosixSignal non-static so pal_process.c can call it. |
| src/native/libs/System.Native/pal_process.h | Updates WaitPidExitedNoHang signature and documents new out parameter semantics. |
| src/native/libs/System.Native/pal_process.c | Populates terminating-signal info when reaping children. |
| src/native/libs/System.Native/pal_process_wasi.c | Updates WASI stub signature for the new out parameter. |
| src/libraries/Common/src/Interop/Unix/System.Native/Interop.WaitPid.cs | Updates managed P/Invoke signature to include terminating signal out parameter. |
| src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs | Switches cached exit info from int? to ProcessExitStatus? and plumbs signal data. |
| src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs | Consumes ProcessExitStatus? and continues populating Process.ExitCode. |
Comments suppressed due to low confidence (2)
src/native/libs/System.Native/pal_process.c:961
- If the
statusis neitherWIFEXITEDnorWIFSIGNALED, the code onlyassert(false)and may leaveexitCode/terminatingSignaluninitialized in release builds. Even if this path is expected to be unreachable withWNOHANG, it should still write safe values (and/or translate it to an error) before returning.
else
{
assert(false);
}
src/native/libs/System.Native/pal_process.c:940
- Consider initializing
*exitCode/*terminatingSignalto deterministic defaults at the start of this function (before callingwaitpid). Whenwaitpidreturns0or-1, the current implementation returns without writing either out parameter; even if callers typically ignore them in those cases, the marshaller still copies them back to managed code.
int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode, int32_t* terminatingSignal)
{
assert(exitCode != NULL);
assert(terminatingSignal != NULL);
int32_t result;
int status;
while (CheckInterrupted(result = waitpid(pid, &status, WNOHANG)));
| @@ -338,7 +338,7 @@ internal bool GetExited(out int? exitCode, bool refresh) | |||
|
|
|||
| // We now have an up-to-date snapshot for whether we've exited, | |||
| // and if we have, what the exit code is (if we were able to find out). | |||
There was a problem hiding this comment.
This comment still refers to caching the "exit code", but the method now tracks ProcessExitStatus?. Update the wording to reflect that we cache the exit status (and that it may be null when it can't be determined).
| // and if we have, what the exit code is (if we were able to find out). | |
| // and if we have, what the exit status is (which may be null if it couldn't be determined). |
|
|
||
| int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode) | ||
| int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode, int32_t* terminatingSignal) | ||
| { |
There was a problem hiding this comment.
SystemNative_WaitPidExitedNoHang returns -1 here without initializing exitCode / terminatingSignal (and without setting errno). Since these are out parameters, managed marshalling will still read back whatever is in those stack locations, which can lead to nondeterministic values if this stub is ever invoked. Initialize both outputs (e.g., to 0) and set errno = ENOTSUP (or the appropriate error) before returning.
| { | |
| { | |
| assert(exitCode != NULL); | |
| assert(terminatingSignal != NULL); | |
| *exitCode = 0; | |
| *terminatingSignal = 0; | |
| errno = ENOTSUP; |
|
@adamsitnik, I'm not reviewing anything today. You can include it in the other PR. It makes more clear why this is being refactored. |
Description
Changes
ProcessWaitStateto store aProcessExitStatus(with signal information) instead of a bareint?exit code, as groundwork for upcoming APIs (#126293).Native (
System.Native)SystemNative_WaitPidExitedNoHangwith a newint32_t* terminatingSignalout parameterWIFSIGNALED, extracts the signal viaWTERMSIGand maps it toPosixSignalusingTryConvertSignalCodeToPosixSignal; on normal exit, setsterminatingSignalto 0TryConvertSignalCodeToPosixSignalnon-static inpal_signal.cand declared it inpal_signal.hsopal_process.ccan call itpal_process.h,pal_process_wasi.csignatures accordinglyManaged
Interop.Sys.WaitPidExitedNoHangto includeout int terminatingSignalProcessWaitState._exitCode(int?) →_exitStatus(ProcessExitStatus?)GetExited,ChildReaped,TryReapChild, andCheckChildrento propagate signal information throughProcessExitStatusProcess.Unix.cs.UpdateHasExitedto consumeProcessExitStatus?