RFR: 8339487: ProcessHandleImpl native exceptions - include message text and last error information [v3]

[Alan Bateman]

On Fri, 6 Sep 2024 07:05:22 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> When running jtreg test java/lang/ProcessHandle/PermissionTest.java on macOS, a few times this error occurs :
>> 
>> java.lang.RuntimeException: Cannot allocate memory
>>        at java.base/java.lang.ProcessHandleImpl.getProcessPids0(Native Method)
>>        at java.base/java.lang.ProcessHandleImpl.children(ProcessHandleImpl.java:456)
>>        at java.base/java.lang.ProcessHandleImpl.children(ProcessHandleImpl.java:434)
>>        at PermissionTest.childrenWithPermission(PermissionTest.java:84)
>>        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
>>        at java.base/java.lang.reflect.Method.invoke(Method.java:573)
>>        at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
>>        at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
>>        at org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
>>        at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
>>        at org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
>>        at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
>>        at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
>>        at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
>>        at java.base/java.util.ArrayList.forEach(ArrayList.java:1597)
>> 
>> 
>> Probably sysctl fails here, but it is not fully clear; it would help to change the exception so that the standard text is shown too in the exception message.
>
> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
> 
>   check for ENOMEM

src/java.base/macosx/native/libjava/ProcessHandleImpl_macosx.c line 128:

> 126:     if (errsysctl < 0) {
> 127:         JNU_ThrowByNameWithMessageAndLastError(env,
> 128:             "java/lang/RuntimeException", "sysctl failed to get info about all processes");

sysctl(3) documents the return as 0 or -1.  So maybe better if the while condition checks errsysctl == -1 and the if condition checks  errsysctl != 0. Otherwise I think this looks good and will be interesting to see if you have other sightings due to other errors.

I guess we should change the JBS/PR title to make it clearer that this issue is now about handling ENOMEM when the number of processes increases. We'll also need to create another issue for the RuntimeException throwing as that will need updates to the ProcessHandle to specify possible error/exceptions.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20839#discussion_r1746669190