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 1502: > 1500: return parseLegacy(servicesMap, ks, vs, opType); > 1501: } else if (value != null && oldValue instanceof String > oldValueS && > 1502: opType == OPType.ADD) { Which method is this else-block for? `value` is not null and not instanceof `String` and `oldValue` is instanceof `String` and `opType` is ADD? src/java.base/share/classes/java/security/Provider.java line 1508: > 1506: // to a removal. In any case, let the caller proceed > with the > 1507: // Properties map update. > 1508: parseLegacy(servicesMap, ks, oldValueS, OPType.REMOVE); no return here and falls through to the line 1512? Is this really intended? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22613#discussion_r1982749780 PR Review Comment: https://git.openjdk.org/jdk/pull/22613#discussion_r1982751287
