On Fri, 17 Jun 2022 11:21:49 GMT, Сергей Цыпанов <d...@openjdk.org> wrote:
>> If there are two threads calling `Executable.hasRealParameterData()` under
>> race and the first one writes into volatile `Executable.parameters` field
>> (doing _releasing store_) and the second thread reads non-null value from
>> the same field (doing acquiring read) then it must read exactly the same
>> value written into `hasRealParameterData` within `privateGetParameters()`.
>> The reason for this is that we assign `hasRealParameterData` before
>> _releasing store_.
>>
>> In the opposite case (_acquiring read_ reads null) the second thread writes
>> the value itself and returns it from the method so there is no change in
>> behavior.
>
> Сергей Цыпанов has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8288327: Mark ParameterData.parameters as @Stable and rename real -> isReal
src/java.base/share/classes/java/lang/reflect/Executable.java line 413:
> 411: }
> 412:
> 413: boolean hasRealParameterData() {
Since `privateGetParameters` and `hasRealParameterData` are now both simple
methods with only one use site, they can be simply inlined in `getParameters()`
and `getAllGenericParameterTypes()` instead:
public Parameter[] getParameters() {
// Need to copy the cached array to prevent users from messing
// with it. Since parameters are immutable, we can
// shallow-copy.
return parameterData().parameters.clone();
}
...
final boolean realParamData = parameterData().isReal();
final Type[] genericParamTypes = getGenericParameterTypes();
final Type[] nonGenericParamTypes = getParameterTypes();
src/java.base/share/classes/java/lang/reflect/Executable.java line 419:
> 417: private ParameterData parameterData() {
> 418: ParameterData parameterData = this.parameterData;
> 419: if (parameterData != null){
Suggestion:
if (parameterData != null) {
src/java.base/share/classes/java/lang/reflect/Executable.java line 777:
> 775: }
> 776:
> 777: record ParameterData(@Stable Parameter[] parameters, boolean isReal)
> {}
I recommend declaring this right next to `parameterData` field for the ease of
maintenance, like how `EntrySet` in map implementations are declared right next
to `entrySet` methods.
-------------
PR: https://git.openjdk.org/jdk/pull/9143
