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

[Koushik Muthukrishnan Thirupattur]

On Mon, 26 Jan 2026 11:08:10 GMT, Mikhail Yankelevich <myankelevich 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 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

Good suggestion. But I am thinking to keep addIfNotPresent() as a generic cache helper without specific construction logic inside.

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

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