Repository navigation
Prevent ClassLatch from latching wrappers with defaults methods - #12823
gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
🟡 Java Benchmark SLOs — Performance SLO warning (near threshold)
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
3f106a9 to
15910e3
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
More details
The message-less AbstractMethodError fallback preserves wrappers with an inherited concrete default method, preventing delegate failures from disabling healthy instances of the same class. The completed static review found no reportable defect.
🤖 Bits Code Review · Commit 15910e3
dougqh
left a comment
There was a problem hiding this comment.
Thanks for fixing my oversight
What Does This Do
Prevents an
AbstractMethodErrorthrown by a wrapper's delegate from latching the wrapper class when interface "evolution" exposes an abstract declaration alongside an inherited default implementation.Motivation
On Java 8, these delegate failures have no message. The fallback added in #12799 inspected only
abstractpublicmethods, which could treat the wrapper as missing the method and disable calls for every healthy instance of that class.Granted, this case seems a tad exotic, but its fix looks easy enough to handle that case.
Suppose the following, in version
A, compiled together.Then there's version
BofApiwith a new method that has the same signature asForwarding's default method, while the existingWrapper.classremains as is:When invoking
wrapper.read()the JVM would still callForwarding.read(). However,Wrapper.class.getMethods()now exposes both:public abstract String Api.read()public default String Forwarding.read()With the following latch
Upon error the cache sees the abstract
Api.read()method and concludes thatWrapperlacks aread()method. It overlooked the workingForwarding.read(), so this permanently disables the operation for everyWrapper.The explicit throw keeps this example small; the reproduction I ran obtained the message-less error from the JVM. This requires keeping the old wrapper binary: recompiling it against the upgraded Api would reject the conflicting declarations.
Additional Notes
The cache remains a
String[]. The fix filters the array in place instead of building aHashSet, so it preserves the existing allocation shape. It adds a second bounded scan only when the cache is built after the first message-less failure for a class. A temporary Java 8 JMH comparison found the array scan faster than theHashSetvariant on method-heavy types. Healthy and cached paths are unchanged.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue