RFR: 8309667: TLS handshake fails because of ConcurrentModificationException in PKCS12KeyStore.engineGetEntry [v3]

[Weijun Wang]

On Fri, 16 Jun 2023 09:25:30 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   more cases to cover
>
> src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 1438:
> 
>> 1436: 
>> 1437:         if (entry.attributes == null) {
>> 1438:             entry.attributes = ConcurrentHashMap.newKeySet();
> 
> It seems that there is still a risk that two threads may compete here, then you may end up with one thread working with a different copy of the attributes, no longer tied to the entry. E.g:
> 
> Thread#1 sees attributes is null, Thread#2 sees attributes is null, both set attributes and only one of them win. The caller in the second thread (that lost) is given the "dangling" map that has not been set, and if it modifies it then modifications will be lost.
> I don't know if this scenario can actually happen - but the possibility is there. Unless there's always some locking further up the stack that would prevent this?
> 
> Also I see in total four places that do:
> 
> [entry|this].attributes = new HashSet<>();
> 
> 
> wouldn't you need to modify all four places in the same manner?

Correct. Done.

I'll look into making the classes immutable in the next release. I hesitate to make such a big change in RDP.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14506#discussion_r1232205474