On Thu, 23 Nov 2023 11:37:12 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> 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?
> An exception here will prevent the class from being initialized...
Maybe we can break it, but this new property is following the same pattern I
think as the neighbouring file
src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPChannel.java when it reads
the system props. I see Integer.getInteger handles the obvious Exceptions, so
specifying a strange value does not immediately break us.
If there's more protection needed then we have various other places to apply it
that might need separate follow-up if you think there's a case?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16771#discussion_r1403331157
