RFR: 8211851: (ch) java/nio/channels/AsynchronousSocketChannel/StressLoopback.java times out (aix) [v2]

[Joachim Kern]

On Mon, 16 Dec 2024 14:02:21 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> Joachim Kern has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   use IOUtil.configureBlocking
>
> src/java.base/aix/classes/sun/nio/ch/AixPollPort.java line 32:
> 
>> 30: import sun.nio.ch.Pollset;
>> 31: import sun.nio.ch.IOUtil;
>> 32: import java.io.FileDescriptor;
> 
> Imports are typically sorted alphabetically.

Done

> src/java.base/aix/classes/sun/nio/ch/AixPollPort.java line 310:
> 
>> 308:             try {
>> 309:                 for (;;) {
>> 310:                     int n, m;
> 
> I'd declare `m` in the `try` block. It's not used outside of it.

Done

> src/java.base/aix/classes/sun/nio/ch/AixPollPort.java line 322:
> 
>> 320:                             // the file descriptor here.
>> 321:                             if (fd != sp[0] && fd != ctlSp[0]) {
>> 322:                                 synchronized (controlQueue) {
> 
> Ok, the lock order `controlLock` before `controlQueue` seems to be consistent. Otherwise, we might get deadlocks. (Exception is `controlLock.tryLock()` which is ok.)
> Shouldn't `synchronized (controlQueue)` better get removed from `processControlQueue()`? The lock has always been acquired before it is called. If it didn't, it would make the lock order inconsistent and possibly cause deadlocks.

Removed the additional` synchronized (controlQueue)` from `processControlQueue()`

> src/java.base/aix/native/libnio/ch/Pollset.c line 170:
> 
>> 168: 
>> 169:     for (;;) {
>> 170:         int n = read(fd, buf, sizeof(buf));
> 
> Shouldn't we handle `errno == EINTR`? Other code uses `RESTARTABLE`.

removed own implementation of `drain()` and use the IOUtil version instead.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22633#discussion_r1888122978
PR Review Comment: https://git.openjdk.org/jdk/pull/22633#discussion_r1888123236
PR Review Comment: https://git.openjdk.org/jdk/pull/22633#discussion_r1888124420
PR Review Comment: https://git.openjdk.org/jdk/pull/22633#discussion_r1888125816