On Tue, 8 Dec 2020 19:40:32 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> This change have been already reviewed by Mandy, Sundar, Alan and David. >> 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. > >> This change have been already reviewed by Mandy, Sundar, Alan and David. >> Now, the PR approval is needed. > > Can you provide a link to the discussion? I'm mostly curious if there was > some discussion as to why Instrument purposefully allowed non-public premain > methods: > > // the premain method should not be required to be public, > 508 // make it accessible so we can call it > 509 // Note: The spec says the following: > 510 // The agent class must implement a public static premain > method... > 511 setAccessible(m, true);``` All the discussion is in the bug and CSR: https://bugs.openjdk.java.net/browse/JDK-8165276 https://bugs.openjdk.java.net/browse/JDK-8248189 We messed up in JDK-5070281 (JDK 6) and it came to light in JDK 9 when auditing the use of setAccessible in the JDK. ------------- PR: https://git.openjdk.java.net/jdk/pull/1694
