On Thu, 10 Dec 2020 00:37:14 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> This change have been already reviewed by Mandy, Sundar, Alan and David. >> Please, see the jdk 15 review thread: >> >> http://mail.openjdk.java.net/pipermail/serviceability-dev/2020-June/031998.html >> >> Now, the PR approval is needed. >> The push was postponed because the CSR was not approved at that time (it is >> now): >> https://bugs.openjdk.java.net/browse/JDK-8248189 >> Investigation of existing popular java agents was requested by Joe. >> ---------- >> >> The java.lang.instrument spec: >> >> https://docs.oracle.com/en/java/javase/15/docs/api/java.instrument/java/lang/instrument/package-summary.html >> >> Summary: >> The java.lang.instrument spec clearly states: >> "The agent class must implement a public static premain method >> similar in principle to the main application entry point." >> Current implementation of sun/instrument/InstrumentationImpl.java >> allows the premain method be non-public which violates the spec. >> This fix aligns the implementation with the spec. >> >> Testing: >> A mach5 run of jdk_instrument tests is in progress. > > Serguei Spitsyn has updated the pull request incrementally with one > additional commit since the last revision: > > removed lookup for inherited premain/agentmain methods Please update @bug in the tests to include this bug ID. All InheritAgentXXX tests are updated to have the InheritAgentXXX class public. However, I think you need to go through them individually for this behavioral change. The existing comments may not be applicable and please update them where appropriate. For example InheritAgent0100 previously verifies the invocation of the premain in its superclass. Does the modified test ensure that this fails to load? Per the comment in the new InheritAgent0100Super class, it expects the superclass' premain should be called. src/java.instrument/share/classes/sun/instrument/InstrumentationImpl.java line 499: > 497: // reject non-public premain or agentmain method > 498: if (!m.canAccess(null)) { > 499: String msg = "method " + classname + "." + methodname + " > must be declared public"; The comments in line 429-443 need to be updated. ------------- PR: https://git.openjdk.java.net/jdk/pull/1694
