RFR: 8345954: Revisit Class Initializers and Locking in X509TrustManagerImpl

[Weijun Wang]

On Tue, 13 Jan 2026 04:14:22 GMT, Koushik Muthukrishnan Thirupattur <duke at openjdk.org> wrote:

> Refactor sun.security.provider.X509Factory cache access to avoid coarse-grained locking and reduce contention during certificate/CRL interning and parsing.
> 
> As per request in [the PR](https://github.com/openjdk/jdk/pull/22616#issuecomment-2524971845), re-visit "the initialisation and locking in this area, e.g. addToCache is a static synchronized method so very coarse locking."

Some comments.

src/java.base/share/classes/sun/security/provider/X509Factory.java line 94:

> 92:             synchronized (certCache) {
> 93:                 certCache.clear();
> 94:             }

This is just for debugging. Maybe not worth any protection.

src/java.base/share/classes/sun/security/provider/X509Factory.java line 183:

> 181:         X509CertImpl newC = isImpl ? (X509CertImpl) c : new X509CertImpl(encoding);
> 182:         byte[] enc = isImpl ? encoding : newC.getEncodedInternal();
> 183:         return addToCache(certCache, enc, newC);

Is this better than the old code? `isImpl` was only checked once.

src/java.base/share/classes/sun/security/provider/X509Factory.java line 230:

> 228:      * Add the X509CertImpl or X509CRLImpl to the cache.
> 229:      */
> 230:     private static <V> V addToCache(Cache<Object, V> cache, byte[] encoding, V value) {

Now that you added double check inside this method to ensure no cache, does it make sense to rename the method to be something like `addIfNotPresent`?

src/java.base/share/classes/sun/security/provider/X509Factory.java line 245:

> 243:     }
> 244: 
> 245: 

Newline?

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

PR Review: https://git.openjdk.org/jdk/pull/29181#pullrequestreview-3692030795
PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2716545800
PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2716567416
PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2716621763
PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2716550443