RFR: 8278461: Use Executable.getSharedParameterTypes() instead of Executable.getParameterTypes() in trusted code

Claes Redestad redestad at openjdk.java.net
Thu Jan 6 16:48:20 UTC 2022


On Thu, 9 Dec 2021 11:50:50 GMT, Сергей Цыпанов <duke at openjdk.java.net> wrote:

> `Executable.getParameterTypes()` creates a copy of underlying array which is redundant in trusted code when we are sure the action is read-only.

Changes requested by redestad (Reviewer).

src/java.base/share/classes/java/lang/reflect/Constructor.java line 374:

> 372:         sb.append('(');
> 373:         StringJoiner sj = new StringJoiner(",");
> 374:         for (Class<?> parameterType : getSharedParameterTypes()) {

Good.

src/java.base/share/classes/java/lang/reflect/Executable.java line 313:

> 311:         // getParameterTypes().
> 312:         if (!genericInfo) {
> 313:             return getSharedParameterTypes();

Since this returns the trusted array it delegates responsibility to the callers of `getAllGenericParameterTypes` to not mutate or further expose/leak the parameter type array. This needs to at least be called out in the method specification. The comment here needs to be updated, as well. Is the added fragility in this case worth the performance win?

src/java.base/share/classes/java/lang/reflect/Executable.java line 317:

> 315:             final boolean realParamData = hasRealParameterData();
> 316:             final Type[] genericParamTypes = getGenericParameterTypes();
> 317:             final Type[] nonGenericParamTypes = getSharedParameterTypes();

(If removing these allocations is important, I note that `ConstructorRepository.getGenericParameterTypes()` lacks an equivalent to `getSharedParameterType` for trusted callers)

Similarly here I'm not sure the win from avoiding the clone is worth delegating the additional responsibility. You could still do this here if you add `.clone()` after `nonGenericParamTypes` on L344

src/java.base/share/classes/java/lang/reflect/Method.java line 434:

> 432:     String toShortSignature() {
> 433:         StringJoiner sj = new StringJoiner(",", getName() + "(", ")");
> 434:         for (Class<?> parameterType : getSharedParameterTypes()) {

Good.

-------------

PR: https://git.openjdk.java.net/jdk/pull/6782


More information about the core-libs-dev mailing list