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

[Koushik Muthukrishnan Thirupattur]

On Thu, 29 Jan 2026 19:47:56 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 242:
> 
>> 240:      * Add the X509CertImpl or X509CRLImpl to the cache.
>> 241:      */
>> 242:     private static <V> V addIfNotPresent(Cache<Object, V> cache, byte[] encoding, V value) {
> 
> Why do we need this method? How is it different from simply calling `put` if both current cache value and possibly-present-in-the-cache value are identical?

I think `put() `can overwrite an entry if two threads race. `addIfNotPresent()` makes the intent explicit: use the already-cached instance if present, otherwise add. That keeps interning correct and avoids unnecessary duplicate objects.

> src/java.base/share/classes/sun/security/provider/X509Factory.java line 413:
> 
>> 411:                 // Build outside lock
>> 412:                 X509CRLImpl crl = new X509CRLImpl(encoding);
>> 413:                 byte[] enc = crl.getEncodedInternal();
> 
> How `enc` is different from  `encoding`? Aren't they the same?

They might be same sometimes but they are not guaranteed to be. `enc` is the canonical encoding produced by the parsed object (`getEncodedInternal()`), which we use as the cache key to normalize equivalent inputs and avoid duplicate cache entries.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2743894042
PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2743895804