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

Alexey Ivanov aivanov at openjdk.org
Fri Feb 28 18:52:52 UTC 2025


On Fri, 28 Feb 2025 18:44:14 GMT, Chen Liang <liach at openjdk.org> wrote:

>> I think this is confusing… If `CACHE.get(signature)` always returns a non-null value, there's no need for the condition `method != null` — it's always true.
>> 
>> The javadoc for the `Cache` class specifies a `null` value can be returned:
>> 
>> https://github.com/openjdk/jdk/blob/e98df71d9c5120fbb73a4c2f49863775fe5db781/src/java.desktop/share/classes/com/sun/beans/util/Cache.java#L98-L99
>> 
>> Is it a bug in `Cache` class that it creates the value and adds it to the cache when the value isn't found?
>> 
>> https://github.com/openjdk/jdk/blob/e98df71d9c5120fbb73a4c2f49863775fe5db781/src/java.desktop/share/classes/com/sun/beans/util/Cache.java#L126-L127
>
> Sorry for my last premature comment.
> 
> I have looked at `Cache`, and believe `create` should be made protected to indicate it is not intended to be called, but only overridden.
> 
> The preexisting code called `create` to reuse the creation mechanism without going through the cache - such usage is dubious. Per my experience, similar conditional caches are better implemented by enclosing the logic into the cache structure and use a common endpoint to access, so we have one universal site to determine the conditionality of caching. In this case, such a condition is better included in the `Cache` class itself, and the `create` method should not be publicly exposed.

Yes, I'd prefer `CACHE.get(signature)`.

At the same time, I've got doubts that Alexander has…

Although, strictly speaking, it is not “implementation dependent” because the `Cache` class is part of OpenJDK, and I think it's reasonable to depend on its implementation.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23845#discussion_r1975876473


More information about the client-libs-dev mailing list