On Wed, 28 Jan 2026 20:16:35 GMT, Artur Barashev <[email protected]> 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

Reply via email to