RFR: 8304932: MethodTypeDescImpl can be mutated by argument passed to MethodTypeDesc.of [v2]
Mandy Chung
mchung at openjdk.org
Mon Apr 17 21:07:44 UTC 2023
On Sun, 26 Mar 2023 20:45:19 GMT, Chen Liang <liach at openjdk.org> wrote:
>> Fixed the bug where if a caller keeps a reference to the array passed into `MethodTypeDesc.of`, the caller may mutate the Desc via the array and can create invalid MethodTypeDesc.
>>
>> Unfortunately, since the input array now needs to be copied, the `of` factory suffers from a performance drop. But otherwise, this patch has minor performance gains on `ofDescriptor` factory, even compared to Adam's patch that optimized `ofDescriptor` in #12945.
>>
>> This patch moves the parameters to an immutable list, to avoid allocations on `parameterList` as well.
>>
>> Benchmark of Oracle JDK 20: https://gist.github.com/683c6219e183cbc2b336224fc2c0d50a
>> Benchmark of this patch: https://gist.github.com/22be9371a2370fb4a7b44f1684750ec4
>> Benchmark of [asotona's patch](https://github.com/openjdk/jdk/pull/12945/files#diff-ac8e413d3e13532a2b0d34a90253c6ddd7a4f04082f792b9d076e9b5a33f2078): https://gist.github.com/eb98579c3b51cafae481049a95a78f80
>>
>> [sotona vs this](https://jmh.morethan.io/?gists=eb98579c3b51cafae481049a95a78f80,22be9371a2370fb4a7b44f1684750ec4); [20 vs this](https://jmh.morethan.io/?gists=683c6219e183cbc2b336224fc2c0d50a,22be9371a2370fb4a7b44f1684750ec4); [20 vs sotona](https://jmh.morethan.io/?gists=683c6219e183cbc2b336224fc2c0d50a,eb98579c3b51cafae481049a95a78f80), for reference
>
> Chen Liang has updated the pull request incrementally with one additional commit since the last revision:
>
> Fix some inconsistencies in the method descriptor derivation methods
Have you considered that the caller of instantiating `MethodTypeDescImpl` is responsible for passing a trusted array? I think that `MethodTypeDescImpl` implementation already assumes it's a trusted array. So `MethodTypeDesc::of` to call `new MethodTypeDescImpl` with a cloned array. `MethodTypeDescImpl` should call `new MethodTypeDescImpl` instead of `MethodTypeDesc::of` as this PR did.
That way will avoid using `JavaUtilCollectionAccess` to create an immutable list without copying the input array.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/13186#issuecomment-1512084178
More information about the core-libs-dev
mailing list