Re: RFR: 8165276: Spec states to invoke the premain method in an agent class if it's public but implementation differs [v11]
On Wed, 23 Dec 2020 18:02:05 GMT, Serguei Spitsyn wrote: >> Thanks for persevering on this one. The updated checks look good. At some >> point I think we re-examine the issue of allowing agents to be non-public, >> maybe emit a warning and eventually disallow it. If/when support is added >> for developing and deployed java agents as modules it might be the time to >> re-examine it. > > Thank you for review, Alan! This PR has been approved by Mandy and Alan. I'm also waiting for approval from David and the CSR approval from Joe. - 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 [v11]
On Wed, 23 Dec 2020 14:06:27 GMT, Alan Bateman wrote: >> Serguei Spitsyn has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 1. Prperly reverted fixes in 3 tests; 2. Got rid of term in >> tests summary comment > > Thanks for persevering on this one. The updated checks look good. At some > point I think we re-examine the issue of allowing agents to be non-public, > maybe emit a warning and eventually disallow it. If/when support is added for > developing and deployed java agents as modules it might be the time to > re-examine it. Thank you for review, Alan! - 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 [v11]
On Tue, 22 Dec 2020 22:11:19 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 incrementally with one > additional commit since the last revision: > > 1. Prperly reverted fixes in 3 tests; 2. Got rid of term in > tests summary comment Thanks for persevering on this one. The updated checks look good. At some point I think we re-examine the issue of allowing agents to be non-public, maybe emit a warning and eventually disallow it. If/when support is added for developing and deployed java agents as modules it might be the time to re-examine it. - Marked as reviewed by alanb (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 [v11]
On Tue, 22 Dec 2020 23:07:12 GMT, Mandy Chung wrote: >> Serguei Spitsyn has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 1. Prperly reverted fixes in 3 tests; 2. Got rid of term in >> tests summary comment > > Thanks for the change. Looks okay. Thank you a lot, Mandy! Sorry for missed changes and overlooked issues in tests. - 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 [v11]
On Tue, 22 Dec 2020 22:11:19 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 incrementally with one > additional commit since the last revision: > > 1. Prperly reverted fixes in 3 tests; 2. Got rid of term in > tests summary comment Thanks for the change. Looks okay. - Marked as reviewed by mchung (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 [v11]
> 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: 1. Prperly reverted fixes in 3 tests; 2. Got rid of term in tests summary comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/1694/files - new: https://git.openjdk.java.net/jdk/pull/1694/files/81cb0e49..219a2cd6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1694=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1694=09-10 Stats: 15 lines in 12 files changed: 0 ins; 0 del; 15 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
