[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8287812: Cleanup JDWP agent GetEnv initialization [v2]
From: Chris Plummer <cjplummer () openjdk ! org>
Date: 2022-12-14 19:38:07
Message-ID: KHVcHaqyWVPNL2-g0apO83GeljCr1DYP-ej5A-L2CVY=.e50b7732-8a55-44dd-962f-bbfe577f01a2 () github ! com
[Download RAW message or body]
On Fri, 9 Dec 2022 01:16:58 GMT, Chris Plummer <cjplummer@openjdk.org> wrote:
> > 3 things to cleanup in this area:
> >
> > (1) The JDWP agent uses `JNI GetEnv(JVMTI_VERSION_1)` to get a JVMTI environment. \
> > If `GetEnv` fails the JDWP agent prints this:
> > `ERROR: JDWP unable to access JVMTI Version 1 (0x30010000), is your J2SE a 1.5 or \
> > newer version? JNIEnv's GetEnv() returned -3`
> > The text "is your J2SE a 1.5 or newer version?" dates from JDK 5 when JVMTI was \
> > introduced and doesn't make sense now.
> > (2) `JVMTI_VERSION_1` suggests that the JDWP agent is looking for a JVMTI v1 \
> > environment when it really wants the latest. `GetEnv` should request \
> > `JVMTI_VERSION` so that it always requests the current version.
> > (3) There is some outdated compatibility checking between runtime and compile \
> > time versions of JVMTI that date back to the 1.0, 1.1, and 1.2 era, and are no \
> > longer needed.
>
> Chris Plummer has updated the pull request incrementally with one additional commit \
> since the last revision:
> Get rid of some structs and statics that are no longer needed.
I'm going to stick with my change to use JVMTI_VERSION rather than JVMTI_VERSION_1. \
It seems that the agent really should be asking for the version it needs, not asking \
for a lesser version. Although the error message might not be as good if someone does \
placement of the debug agent into a different JDK build, no one should be doing this \
in the first place, and the resulting error message is still appropriate.
Thanks for the reviews Alan, Serguei, and Alex!
-------------
PR: https://git.openjdk.org/jdk/pull/11602
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic