On Mon, 3 Mar 2025 18:31:19 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> 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 637:
>
>> 635: // let javadoc show doc from superclass
>> 636: @Override
>> 637: public synchronized Object get(Object key) {
>
> How about the getProperty(String) method on line 675? Add `@Override` tag and
> `synchronized` keyword there also? And the `keySet()` and `values()` methods
> on line 432 and 444 respectively? What is the criteria for synchronizing the
> method of the `Provider` class?
The reason why we need to synchronize `get` is because the handling of the
properties map from ServicesMap may expose temporary holes. For example, if a
property backed up by a service is overwritten with a value that is not (e.g. a
non-string value), we need to delete the service first. When deleting the
service, the properties map will have a temporary hole that is then filled up
with the new property value.
It's possible that we missed some other methods such as `getProperty`. We will
review them again.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22613#discussion_r2027591882
