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

[Koushik Muthukrishnan Thirupattur]

On Wed, 28 Jan 2026 20:16:35 GMT, Artur Barashev <abarashev at openjdk.org> wrote:

> I wonder why we need to do any synchronization in `X509Factory` at all when `sun.security.util.MemoryCache` being used is internally synchronized and its documentation states that it's `safe for concurrent use by multiple threads`:
> 
> https://github.com/openjdk/jdk/blob/7efa3168b706c1d061c4ee65574427ef1f50fc7b/src/java.base/share/classes/sun/security/util/Cache.java#L267
> 
> I think we need to look closer into this.

I think MemoryCache is synchronized for individual get/put/clear, but we still need to synchronize inside addIfNotPresent() because it’s a compound check-then-put and needs atomicity to preserve interning semantics.

> src/java.base/share/classes/sun/security/provider/X509Factory.java line 234:
> 
>> 232:      */
>> 233:     private static <V> V getFromCache(Cache<Object, V> cache, byte[] encoding) {
>> 234:         synchronized (cache) {
> 
> This `synchronized` is redundant. This cache's `get` method is already synchronized.

I had this because synchronizing in getFromCache() makes our locking policy explicit and keeps the access pattern consistent with addIfNotPresent(). But I am okay with removing it.

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

PR Comment: https://git.openjdk.org/jdk/pull/29181#issuecomment-3820814229
PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2743863462