On Tue, 10 Oct 2023 09:36:47 GMT, Joachim Kern <jk...@openjdk.org> wrote:
>> We see rather often failures in java/lang/ProcessHandle/TreeTest.java on AIX
>> in TreeTest.test5.
>> The reason is: Previously the implementation based on the /proc file system
>> lead to double pids in the child list; at least intermittent. Using the API
>> getprocs64() instead I was able to eliminate those double pids (and increase
>> the performance by a factor of 4). Otherwise we would have to add an
>> algorithm to filter out the doubles after creating the raw list.
>>
>> Here is a java callstack of the error situation:
>> test TreeTest.test5(): failure
>> java.lang.AssertionError: expected direct children expected [2] but found [3]
>> at org.testng.Assert.fail(Assert.java:99)
>> at org.testng.Assert.failNotEquals(Assert.java:1037)
>> at org.testng.Assert.assertEqualsImpl(Assert.java:140)
>> at org.testng.Assert.assertEquals(Assert.java:122)
>> at org.testng.Assert.assertEquals(Assert.java:907)
>> at TreeTest.test5(TreeTest.java:447)
>> at
>> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
>> at java.base/java.lang.reflect.Method.invoke(Method.java:580)
>> 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)
>> at org.testng.TestRunner.privateRun(TestRunner.java:764)
>> at org.testng.TestRunner.run(TestRunner.java:585)
>> at org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
>> at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
>> at org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
>> at org.testng.SuiteRunner.run(SuiteRunner.java:286)
>> at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
>> at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
>> ...
>
> Joachim Kern has updated the pull request incrementally with one additional
> commit since the last revision:
>
> cosmetic changes
The full stack trace doesn't add any value in the PR description and makes
every email on the subject too long.
If its in the issue, it doesn't need to be here. $0.02, Roger
src/java.base/aix/native/libjava/ProcessHandleImpl_aix.c line 85:
> 83: const int Chunk = 100;
> 84: struct procentry64 ProcessBuffer[Chunk];
> 85: pid_t IndexPointer = 0;
Initial capital 'I' is unusual for a local variable.
src/java.base/aix/native/libjava/ProcessHandleImpl_aix.c line 106:
> 104: }
> 105:
> 106: while ((num = getprocs64(ProcessBuffer, sizeof(struct
> procentry64), NULL, sizeof(struct fdsinfo64), &IndexPointer, Chunk)) != -1) {
Please line break somewhere between 80-100.
src/java.base/aix/native/libjava/ProcessHandleImpl_aix.c line 108:
> 106: while ((num = getprocs64(ProcessBuffer, sizeof(struct
> procentry64), NULL, sizeof(struct fdsinfo64), &IndexPointer, Chunk)) != -1) {
> 107: for (i = 0; i < num; i++) {
> 108: pid_t ppid = 0;
Indentation should be 4 for core-libs native files.
src/java.base/aix/native/libjava/ProcessHandleImpl_aix.c line 138:
> 136: }
> 137: if (num < Chunk)
> 138: break;
indent = 4
The style (in this file) after `if` is to use braces.
src/java.base/aix/native/libjava/ProcessHandleImpl_aix.c line 155:
> 153: JNU_ThrowByNameWithLastError(env,
> 154: "java/lang/RuntimeException", "Unable to retrieve Process
> info");
> 155: return -1;
Identation = 4
-------------
PR Comment: https://git.openjdk.org/jdk/pull/16051#issuecomment-1756094157
PR Review Comment: https://git.openjdk.org/jdk/pull/16051#discussion_r1353220388
PR Review Comment: https://git.openjdk.org/jdk/pull/16051#discussion_r1353209896
PR Review Comment: https://git.openjdk.org/jdk/pull/16051#discussion_r1353211188
PR Review Comment: https://git.openjdk.org/jdk/pull/16051#discussion_r1353214079
PR Review Comment: https://git.openjdk.org/jdk/pull/16051#discussion_r1353216706
