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

Koushik Muthukrishnan Thirupattur duke at openjdk.org
Fri Jan 30 05:56:15 UTC 2026


On Fri, 30 Jan 2026 00:44:15 GMT, Artur Barashev <abarashev at openjdk.org> wrote:

>> I think we still need `addIfNotPresent` because the earlier `getFromCache()` and the put are not atomic and can race with other threads. `addIfNotPresent` performs the check-then-put under one lock and returns the already-cached instance if another thread won the race. Also, the cache key for insertion is the canonical enc, which may differ from the raw encoding used for the initial lookup, so we might still need this.
>
> But we can simply call `addIfNotPresent` without `getFromCache`, right? I mean we don't need to perform 2 `get` operations in the row.

Good point. Yes, we could call `addIfNotPresent()` directly and drop the initial `getFromCache()`.
I kept the fast-path `getFromCache()` so cache hits don’t have to enter the synchronized block in `addIfNotPresent()`; the “two gets” we worry about here only happens on misses.
Also, we still need addIfNotPresent() after the first check because the miss path is racy and we want the “check-then-insert and return existing” behavior to be atomic, and to converge on the canonical enc key.

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

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


More information about the security-dev mailing list