On Mon, 16 Oct 2023 11:13:57 GMT, Mark Sheppard <mshep...@openjdk.org> wrote:
>> Matthew Donovan has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains four additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into socket-params
>> - Merge branch 'master' into socket-params
>> - added javadocs to new methods
>> - 8303525: Refactor/cleanup
>> open/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java
>
> test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java line 77:
>
>> 75:
>> 76: public void testRmiCommunication(RMIServerSocketFactory
>> serverFactory) throws Exception {
>> 77: Hello stub = (Hello)UnicastRemoteObject.exportObject(new
>> HelloImpl(),
>
> by not retaining an explicit reference to the test rmi server, you are
> exposing it to potentially being GCed during the test execution and
> potentially prior to client invocation... this might sound fanciful but this
> has been observed in a few scenarios due to the GC changes ... althought it
> doesn't seem to have been an issue, the structure of the test appears to be
> inherently racy, with the potential for the client invocation to getting
> ahead of the server with the rmi server launching background threads.
> Anyways, caution on not retaining a sever reference and GC interference.
Yes, this can still happen even if the reference to the server object is held
in a local variable. It's unlikely to occur, but to prevent any possibility of
unexpected GC, you need to use reachabilityFence. Something like:
HelloImpl server = new HelloImpl();
try {
// ... use server ...
} finally {
Reference.reachabilityFence(server);
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14932#discussion_r1364336386
