Re: RFR: 8165276: Spec states to invoke the premain method in an agent class if it's public but implementation differs [v12]
On Sun, 24 Jan 2021 21:10:56 GMT, David Holmes wrote: >> Serguei Spitsyn has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 12 additional >> commits since the last revision: >> >> - Merge >> - 1. Prperly reverted fixes in 3 tests; 2. Got rid of term in >> tests summary comment >> - @summary of some negative tests is corrected >> - 1. Suppress premain method check only if agent class is in unnamed >> module; 2. minor tests cleanup >> - added to NegativeAgentRunner exit value check to be non-zero >> - 1. Replaced AgentJarBuilder with JavaAgentBuilder; 2. Removed unneeded >> sun.tools.jar imports >> - fixed trailing spaces >> - (1) Update premain/agentmain method lookup; (2) refactored tests to get >> rid of shell scripts >> - added 8165276 to @bug list of impacted tests >> - (1)Fix comment in InstrumentationImpl; (2) Update agent super classes in >> tests with new expectations that premain methods in supers should non be >> called >> - ... and 2 more: >> https://git.openjdk.java.net/jdk/compare/77ea10ca...d926e4b1 > > Hi Serguei, > > This seems good to go. > > Thanks, > David Thank you for review, David! - PR: https://git.openjdk.java.net/jdk/pull/1694
Re: RFR: 8165276: Spec states to invoke the premain method in an agent class if it's public but implementation differs [v12]
On Fri, 22 Jan 2021 11:27:38 GMT, Serguei Spitsyn 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 with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 12 additional > commits since the last revision: > > - Merge > - 1. Prperly reverted fixes in 3 tests; 2. Got rid of term in > tests summary comment > - @summary of some negative tests is corrected > - 1. Suppress premain method check only if agent class is in unnamed module; > 2. minor tests cleanup > - added to NegativeAgentRunner exit value check to be non-zero > - 1. Replaced AgentJarBuilder with JavaAgentBuilder; 2. Removed unneeded > sun.tools.jar imports > - fixed trailing spaces > - (1) Update premain/agentmain method lookup; (2) refactored tests to get > rid of shell scripts > - added 8165276 to @bug list of impacted tests > - (1)Fix comment in InstrumentationImpl; (2) Update agent super classes in > tests with new expectations that premain methods in supers should non be > called > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/da29edf6...d926e4b1 Hi Serguei, This seems good to go. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1694
Re: RFR: 8165276: Spec states to invoke the premain method in an agent class if it's public but implementation differs [v12]
> 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last revision: - Merge - 1. Prperly reverted fixes in 3 tests; 2. Got rid of term in tests summary comment - @summary of some negative tests is corrected - 1. Suppress premain method check only if agent class is in unnamed module; 2. minor tests cleanup - added to NegativeAgentRunner exit value check to be non-zero - 1. Replaced AgentJarBuilder with JavaAgentBuilder; 2. Removed unneeded sun.tools.jar imports - fixed trailing spaces - (1) Update premain/agentmain method lookup; (2) refactored tests to get rid of shell scripts - added 8165276 to @bug list of impacted tests - (1)Fix comment in InstrumentationImpl; (2) Update agent super classes in tests with new expectations that premain methods in supers should non be called - ... and 2 more: https://git.openjdk.java.net/jdk/compare/06cc1b7a...d926e4b1 - Changes: - all: https://git.openjdk.java.net/jdk/pull/1694/files - new: https://git.openjdk.java.net/jdk/pull/1694/files/219a2cd6..d926e4b1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1694&range=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1694&range=10-11 Stats: 114388 lines in 3016 files changed: 55977 ins; 36917 del; 21494 mod Patch: https://git.openjdk.java.net/jdk/pull/1694.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1694/head:pull/1694 PR: https://git.openjdk.java.net/jdk/pull/1694
