On Sat, 8 Feb 2025 04:21:20 GMT, Sergey Bylokhov <s...@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+azveg...@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
