On Thu, 29 Jan 2026 19:47:56 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 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
