On Wed, 11 Dec 2024 19:08:52 GMT, Anthony Scarpino <ascarp...@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 >> * `impl` (`ServicesMapImpl`) >> * `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 registration) >> * `Current asCurrent()`: returns `impl` seen as a `Current` >> interface implementer >> * `putService(Service svc)` >> * `removeService(Service svc)` >> * `Legacy asLegacy()`: returns `impl` seen as a `Legacy` interface >> implementer >> * `putClassName(ServiceKey key, String className, String >> propKey)` >> * `putAlias(ServiceKey key, ServiceKey aliasKey, String propKey)` >> * `putAttribute(ServiceKey key, String attrName, String >> attrValue, String propKey)` >> * `remove(ServiceKey key, String className)` >> * `removeAlias(ServiceKey key, ServiceKey aliasKey)` >> * `removeAttribute(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 [... > > src/java.base/share/classes/java/security/Provider.java line 710: > >> 708: * Enum to inform the result of an operation on the services >> map. >> 709: */ >> 710: enum SvcOpResult { > > Why use an enum here when a boolean is sufficient? Just to add some more semantic value but we don't have a strong opinion, we can replace it with boolean if you want. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22613#discussion_r1881378029
