On Thu, 29 Jan 2026 19:53:05 GMT, Artur Barashev <[email protected]> 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

Reply via email to