On Fri, 7 Jun 2024 16:05:06 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:
>> src/java.base/windows/native/libjava/ProcessHandleImpl_win.c line 128:
>>
>>> 126: JNU_ThrowByNameWithLastError(env,
>>> 127: "java/lang/RuntimeException",
>>> "WaitForMultipleObjects");
>>> 128: } else if (!GetExitCodeProcess(handle, &exitValue)) {
>>
>> This can return STILL_ACTIVE if there is a spurious thread interrupt.
>> The interrupt is presumably present to keep the thread from being stuck in a
>> blocking windows os call.
>>
>> The calling code in ProcessHandleImpl is agnostic to platform and would
>> report the process had exited.
>>
>> Can the risk of incorrectly reporting the process exit be mitigated?
>>
>> If the Thread is legitimately been interrupted, Thread.interrupted() would
>> be true and the reaper could exit.
>> If false, it could retry the waitForProcessExit().
>
> I only left it here because the wait for interrupt event was already present.
> Is being stuck in a blocking os call a bad thing? If not, I can drop the
> interrupt event, and wait on the process handle only.
I think waiting on JVM_GetThreadInterruptEvent() is necessary during VM
shutdown to allow the blocked thread to exit cleanly.
>> src/java.base/windows/native/libjava/ProcessImpl_md.c line 471:
>>
>>> 469: {
>>> 470: return WaitForSingleObject((HANDLE) handle, 0) /* don't wait */
>>> 471: == WAIT_TIMEOUT;
>>
>> Would this be better as `isProcessAlive(handle)`?
>
> I don't follow. Could you explain?
The `WaitForSingleObject(handle, 0)` seems like an indirect way to determine if
the process is alive.
Whereas `isProcessAlive(handle)` seems to directly answer the question.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19586#discussion_r1631664185
PR Review Comment: https://git.openjdk.org/jdk/pull/19586#discussion_r1631662973
