On Wed, 12 Feb 2025 20:46:31 GMT, Francisco Ferrari Bihurriet <fferr...@openjdk.org> wrote:
>> Hi, this pull request implements the fixes for bugs and inconsistencies >> described in [JDK-8345139](https://bugs.openjdk.org/browse/JDK-8345139 "Fix >> bugs and inconsistencies in the Provider services map"). >> >> #### New services map design >> >> Here is the high-level hierarchy of the new services map design: >> >> * `servicesMap` (`ServicesMap`) >> * Instances (fields) >> * `services` (`Map<ServiceKey, Service>`): unifies the previous >> `serviceMap` and `legacyMap` >> * `legacySvcKeys` (`Set<ServiceKey>`): set indicating which keys in >> `services` correspond to the Legacy API >> * `serviceProps` (`Map<ServiceKey, String>`): keeps track of the >> _provider Hashtable entries_ that originated services entries <sup>(1)</sup> >> * `serviceAttrProps` (`Map<ServiceKey, Map<UString, String>>`): >> keeps track of the _provider Hashtable entries_ that originated service >> attributes <sup>(1)</sup> >> * `serviceSet` (`AtomicReference<Set<Service>>`): part of a >> lock-free mechanism to implement a consistent version of the `getServices()` >> readers method >> * Writers' methods (for providers registering services through the >> Current or the Legacy API) >> * `boolean putService(Service svc)` >> * `boolean removeService(Service svc)` >> * `boolean putClassNameLegacy(ServiceKey key, String className, >> String propKey)` >> * `boolean putAliasLegacy(ServiceKey key, ServiceKey aliasKey, >> String propKey)` >> * `boolean putAttributeLegacy(ServiceKey key, String attrName, >> String attrValue, String propKey)` >> * `boolean removeLegacy(ServiceKey key, String className)` >> * `boolean removeAliasLegacy(ServiceKey key, ServiceKey aliasKey)` >> * `boolean removeAttributeLegacy(ServiceKey key, String attrName, >> String attrValue)` >> * Readers' methods (for services users and `GetInstance` APIs) >> * `Set<Service> getServices()` >> * `Service getService(ServiceKey key)` >> * Other methods: default and copy constructors, `void clear()` >> >> (1): to maintain the consistency between the provider's `servicesMap` and >> its _Hashtable entries_, even if Legacy API updates occur through >> _properties_ with different casing, or aliases instead of main algorithms. >> >> #### Testing >> >> As part of our testing, we observed all the tests pass in the following >> categories: >> >> * `jdk:tier1` (see [GitHub Actions >> run](https://github.com/franferrax/jdk/actions/runs/12193211398)) >> * `jdk/com/sun/crypto` >> * `jd... > > Francisco Ferrari Bihurriet has updated the pull request incrementally with > one additional commit since the last revision: > > Clear ServicesMap fields in the declared order > > Constructors assign the fields in the same order. src/java.base/share/classes/java/security/Provider.java line 1536: > 1534: > 1535: private Object implRemove(Object key) { > 1536: Object oldValue = super.get(key); `oldValue` isn't really old value, is it? Naming it `oldValue` and pass it to `doLegacyOp()` as `value` is very confusing. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22613#discussion_r1982802851