On Fri, 14 Jul 2023 16:44:10 GMT, Xue-Lei Andrew Fan <[email protected]> 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
