RFR: 8344892: beans/finder/MethodFinder.findMethod incorrectly returns null [v2]
Alexander Zvegintsev
azvegint at openjdk.org
Mon Mar 3 14:36:32 UTC 2025
> 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:
return CACHE.get(signature);
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/23845/files
- new: https://git.openjdk.org/jdk/pull/23845/files/36546eb6..e6f0f433
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=23845&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=23845&range=00-01
Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/23845.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/23845/head:pull/23845
PR: https://git.openjdk.org/jdk/pull/23845
More information about the client-libs-dev
mailing list