RFR: 8344892: beans/finder/MethodFinder.findMethod incorrectly returns null [v3]
Sergey Bylokhov
serb at openjdk.org
Mon Mar 3 20:41:59 UTC 2025
On Mon, 3 Mar 2025 16:52:04 GMT, Alexander Zvegintsev <azvegint at openjdk.org> wrote:
>> During the [JDK-8344891 Remove uses of sun.misc.ReflectUtil in java.desktop](https://bugs.openjdk.org/browse/JDK-8344891) it was discovered that `beans/finder/MethodFinder.findMethod' incorrectly returned null if a signature was not in the cache and added it to the cache if it was already there:
>>
>>
>> Method method = CACHE.get(signature);
>> return (method == null) ? method : CACHE.create(signature);
>>
>> This resulted in a [significant drop in performance](https://bugs.openjdk.org/browse/JDK-8350573).
>>
>> ----
>>
>> Before ReflectUtil was removed, it worked by coincidence:
>>
>>
>> Method method = CACHE.get(signature);
>> return (method == null) || isPackageAccessible(method.getDeclaringClass()) ? method : CACHE.create(signature);
>>
>>
>>
>>
>> 1. `Cache#get` non-obviously creates a value in Cache, this in turn allowed us to avoid the NPE in the `(method == null) || isPackageAccessible(method.getDeclaringClass())` condition
>>
>>
>> https://github.com/openjdk/jdk/blob/d6c4be672f6348f8ed985416ed90d0447f5d5bb3/src/java.desktop/share/classes/com/sun/beans/util/Cache.java#L120-L126
>>
>> 2. `isPackageAccessible(method.getDeclaringClass())` was evaluated as true
>>
>> This is how we previously returned the cached value.
>>
>> ---
>>
>> So the solution is obvious:
>>
>>
>> Method method = CACHE.get(signature);
>> return (method != null) ? method : CACHE.create(signature);
>>
>>
>> Testing is green.
>
> Alexander Zvegintsev has updated the pull request incrementally with one additional commit since the last revision:
>
> year update
src/java.desktop/share/classes/com/sun/beans/finder/MethodFinder.java line 80:
> 78:
> 79: try {
> 80: return CACHE.get(signature);
Is it possible to create a test for this patch?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23845#discussion_r1978162593
More information about the client-libs-dev
mailing list