RFR: 8344892: beans/finder/MethodFinder.findMethod incorrectly returns null [v3]

[Sergey Bylokhov]

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

>Cache#get non-obviously creates a value in Cache

Then why we need to call CACHE.create the second time? Can we just return it as is? 
`return CACHE.get(signature);`

I think previously, we couldn't return the cached data as is, since it could be cached by one applet and requested by another. So, in the case of an inaccessible package, we call CACHE.create() and return a duplicate.

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

PR Comment: https://git.openjdk.org/jdk/pull/23845#issuecomment-2695593424