RFR: 8303525: Refactor/cleanup open/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java [v2]

[Stuart Marks]

On Mon, 16 Oct 2023 11:13:57 GMT, Mark Sheppard <msheppar at 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