RFR: 8345954: Revisit Class Initializers and Locking in X509TrustManagerImpl [v2]

[Koushik Muthukrishnan Thirupattur]

On Thu, 29 Jan 2026 19:53:05 GMT, Artur Barashev <abarashev at openjdk.org> wrote:

>> Koushik Muthukrishnan Thirupattur has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8345954: Addressing review comments
>
> src/java.base/share/classes/sun/security/provider/X509Factory.java line 118:
> 
>> 116:         X509CertImpl newCert = new X509CertImpl(encoding);
>> 117:         byte[] enc = newCert.getEncodedInternal();
>> 118:         return addIfNotPresent(certCache, enc, newCert);
> 
> Same: Why we call `addIfNotPresent` if we already checked above that it's not present?

I think we still need `addIfNotPresent` because the earlier `getFromCache()` and the put are not atomic and can race with other threads. `addIfNotPresent` performs the check-then-put under one lock and returns the already-cached instance if another thread won the race. Also, the cache key for insertion is the canonical enc, which may differ from the raw encoding used for the initial lookup, so we might still need this.

> src/java.base/share/classes/sun/security/provider/X509Factory.java line 414:
> 
>> 412:                 X509CRLImpl crl = new X509CRLImpl(encoding);
>> 413:                 byte[] enc = crl.getEncodedInternal();
>> 414:                 return addIfNotPresent(crlCache, enc, crl);
> 
> Why we call `addIfNotPresent` if we already checked above that it's not present?

Same as above.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2743912878
PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2743914237