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

[Mikhail Yankelevich]

On Thu, 22 Jan 2026 23:51:42 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."
>
> Koushik Muthukrishnan Thirupattur has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8345954: Addressing review comments

Looks fine to me, just one minor question

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

> 186:             enc = newC.getEncodedInternal();
> 187:         }
> 188:         return addIfNotPresent(certCache, enc, newC);

Nit: what do you think about moving this logic inside of the `addIfNotPresent` and just pass an option if it is an impl? 

        X509CertImpl newC;
        byte[] enc;
        if (isImpl) {
            newC = (X509CertImpl) c;
            enc = encoding;
        } else {
            newC = new X509CertImpl(encoding);
            enc = newC.getEncodedInternal();
        }
        return addIfNotPresent(certCache, enc, newC);


I think this might be a bit easier to follow then. But I don't really mind if you leave it this way either

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

PR Review: https://git.openjdk.org/jdk/pull/29181#pullrequestreview-3705573013
PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2727198948