On Thu, 23 Nov 2023 11:35:11 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> RMI Connections (in general) should use a timeout on the Socket connect call >> by default. >> >> JMX connections use RMI and some connection failures never terminate. The >> hang described in 8316649 is hard to reproduce manually: the description >> says it can be caused by a firewall that never returns a response. >> >> src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPChannel.java >> has other timeouts but nothing to control the initial Socket connect. >> >> Defaulting to a 1 minute timeout on connect has no effect on existing tests >> for RMI and JMX, and should go unnoticed in applications unless there really >> is a significant connection delay. Specifying zero for the new System >> Property sun.rmi.transport.tcp.initialConnectTimeout uses the old code path >> of a connect with no timeout. >> >> I have a test, but it is not realistically usable: although specifying a 1 >> millisecond timeout will often fail (as expected/desired for the test), it >> will often pass as the connection happens immediately. > > src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPDirectSocketFactory.java > line 46: > >> 44: private static final int connectTimeout = // default 1 minute >> 45: AccessController.doPrivileged((PrivilegedAction<Integer>) () -> >> 46: >> Integer.getInteger("sun.rmi.transport.tcp.initialConnectTimeout", 60 * >> 1000).intValue()); > > An exception here will prevent the class from being initialized, any > subsequent attempts to use the class will then produce hard to diagnose > error. If the class is not loaded by the main thread, such exception/error > could be swallowed unless an uncaught exception handler was registered. I > would suggest implementing the logic in a static block, catch any exception > and revert to default behaviour (i.e. 0) if the value supplied for the > property is not an integer, or not a valid integer, etc... IIRC, the default agent uses / may use its own socket factories - are we still coming here even with those factories? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16771#discussion_r1403260460