RFR: 8347826: Introspector shows wrong method list after 8071693 [v4]

[Sergey Bylokhov]

On Sat, 8 Feb 2025 04:21:20 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> Roman Marchenko has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update test/jdk/java/beans/Introspector/DefaultMethodBeanPropertyTest.java
>>   
>>   Co-authored-by: Aleksandr Zvegintsev <77687766+azvegint at users.noreply.github.com>
>
> In jdk 9 we started to sort the list of methods for each class for a few reasons, and the main one was:
>  1. We had a number of bugs which state that our JavaBeans randomly does not work, examples: JDK-6807471[1] , JDK-6788525[2], the reason was that the order of methods from Class.getMethods() is not specified.
> 
> So MethodOrder is just a workaround to help us reproduce the bugs. I understand how the current change will fix the problem, but it would be nice to update the code that retrieves the method from that list, or at least provide some information why we can't do that.
> 
> Example from the past https://github.com/openjdk/jdk/pull/7190 where MethodOrder was simplified due to JDK-8196373

> @mrserb Sorry, I don't feel I completely understand what are you asking for. What do you mean "the code that retrieves the method from that list"? Should I add comments somewhere in source code with the issue details why does Introspector.addMethod() work wrong for now, or with details about sorting? Or should I update the PR description? I couldn't find any clues in #7190, sorry.

In the past we received various reports that were caused by bugs in the Introspector implementation, but we could not reproduce them because that bugs depended on some order of methods returned by the Class.getMethods().

So, the method you updated was added to make the method list stable, it is just a utility wrapper. You don't need to update that method.

But instead you can change the code that uses the result of MethodInfo.get() in `PropertyInfo.get()` or `PropertyInfo.initialize()` where we created the property to pick the correct default/non-default method.

Or, most likely, you can skip adding the default method in MethodInfo.get() if non-default methods are already added.

> 
> And BTW, I'm concerning why I can't find a check in this PR which builds and tests the change. Should I trigger it somehow, or is it just invisible for me?

You should enable that actions/checks in your fork.

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

PR Comment: https://git.openjdk.org/jdk/pull/23443#issuecomment-2649725000