RFR: 8290005: com/sun/jndi/ldap/LdapCBPropertiesTest.java failling with NullPointerException [v2]

[Matthew Donovan]

On Fri, 14 Jul 2023 16:44:10 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> The only lock I added was in `TransportContext` to synchronize access to the `handshakeContext` field, but I understand your reluctance to make any changes with locks. The problem is that SSLSocketImpl tries to access `conContext.handshakeContext.negotiatedProtocol` after TransportContext has set handshakeContext to null.
>> 
>> TransportContext also has a `protocolVersion` field. Is it possible to just use that instead?
>
>> TransportContext also has a `protocolVersion` field. Is it possible to just use that instead?
> 
> Did you mean a change in duplexCloseOutput() like the following?
> 
> 
> -           // The protocol version may have been negotiated.
> -           ProtocolVersion pv = conContext.handshakeContext.negotiatedProtocol;
> +           // The protocol version may have been negotiated.  The 
> +           // conContext.handshakeContext.negotiatedProtocol is not used as there
> +           // may be a race to set it to null.
> +          ProtocolVersion pv = conContext.protocolVersion;
>             if (pv == null || (!pv.useTLS13PlusSpec())) {
>                 hasCloseReceipt = true;
>             }       
> 
> 
> I like the idea as it looks like a safe update in the current implementation.

Yes, that is what I was thinking.I will make the change and test it out. Thanks.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13742#discussion_r1264004624