RFR: 8287812: Cleanup JDWP agent GetEnv initialization [v2]

[Alan Bateman]

On Sat, 10 Dec 2022 10:29:43 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> I have a memory of the following concerning JVM/TI versions:
>> 
>> - if you asked for JVMTI_VERSION, then you got the highest version supported by the JVM.
>> - if you asked for JVMTI_VERSION_1, then you got the highest compatible version to VERSION_1.
>> - if you asked for JVMTI_VERSION_1_1, then you got JVMTI_VERSION_1_1, no more, no less if the JVM supports it.
>> 
>> Here's where things get gnarly:
>> - if you asked for JVMTI_VERSION_1, could you get back JVMTI_VERSION_1_2 or JVMTI_VERSION_9 or
>>   JVMTI_VERSION_11 if the JVM in question supported each of those versions (and no higher)?
>>   - the assumption that I've always made is that each of JVMTI_VERSION_1_N are all considered
>>     compatible so asked for JVMTI_VERSION_1, you would get the highest compatible version in
>>     the JVMTI_VERSION_1_N set that is supported by the JVM on which you ask the question.
>>   - if you ask for JVMTI_VERSION_1 on a JVM that supports JVMTI_VERSION_1_N and JVMTI_VERSION_9
>>     then I would expect you to get back JVMTI_VERSION_1_2 and not JVMTI_VERSION_9. Why?
>>   - My assumption is that you only change major version numbers when you make incompatible
>>     changes. Adding a new API is a compatible change because the older agent doesn't know how
>>     to use the new API. Changing the semantics of an existing API is NOT a compatible change so
>>     you bump the major number.
>> 
>> So I'm assuming that JVMTI_VERSION_9 has incompatible changes relative to JVMTI_VERSION_1_N.
>> Similarly I'm assuming that JVMTI_VERSION_11 has incompatible changes relative to JVMTI_VERSION_9
>> and JVMTI_VERSION_1_N.
>
>> I have a memory of the following concerning JVM/TI versions:
>> 
>> * if you asked for JVMTI_VERSION, then you got the highest version supported by the JVM.
> 
> JVMTI_VERSION is defined in jvmti.h so its value depends on which JDK include directory was used when compiling the agent. If the JDK supports that version then it is required to return a JVMTI env that is compatible or fail with EVERSION. If the agent calls GetVersionNumber to see what version it got then it may be a newer (but compatible) version, which is what our implementation does. It may be that the agent was compiled with JVMTI_VERSION for a newer JDK release, in which case GetEnv must fail with EVERSION as an older JDK release doesn't know about new JVMTI versions.
> 
> In any case, there should be no mix'ing and matching with the JDWP agent so GetEnv asking for JVMTI_VERSION should be okay. For a JDK build, the JVMTI version at build time will match run-time. A hash of the jdk.jdwp.agent module is generated at build/packaging time to prevent accidental linking with modules from different JDK builds. If someone does attempt to run jlink and use java.base from one build and jdk.jdwp.agent from another build then they will get an error that the hashes don't match. It is of course possible that someone does a slight of hand and copy a libjdwp/libdt_socket from one JDK build into a build from a different JDK release but I don't think we need to spend too much time on that as it's just not supported to do that. Also, as Chris says, the version compatibility check should catch it too.
> 
> 
>> * if you asked for JVMTI_VERSION_1, then you got the highest compatible version to VERSION_1.
>> * if you asked for JVMTI_VERSION_1_1, then you got JVMTI_VERSION_1_1, no more, no less if the JVM supports it.
> 
> If you run on JDK 19 then you'll get a JVMTI_VERSION_19 in both cases. It might be in the future that we need to do some incompatible changes to JVMTI, like remove the deprecated heap functions, in which case it might have to return EVERSION for both cases. The is a bridge that we haven't got to yet but I expect it will require discussing JVMTI capabilities at the same time.
> 
> In passing, I see that JvmtiExport::get_jvmti_interface doesn't reject 19.minor, I guess it should.

> @AlanBateman I agree with everything you say, but there is one advantage to using JVMTI_VERSION_1 instead of JVMTI_VERSION. With the former if you tried, for example, to drop the JDK 21 debug agent into a JDK 20 build, you will get:
> 
> `ERROR: This jdwp native library will not work with this VM's version of JVMTI (20.0.0). It needs JVMTI 21.0[.0].`
> 
> However, if you use JVMTI_VERSION, the error you will see is:
> 
> `ERROR: JDWP unable to access JVMTI Version 21.0.0 (0x30150000). JNIEnv's GetEnv() returned -3.`
> 
> This is because the first example passes the GetEnv() but then fails the version check, and the 2nd one fails the GetEnv(). Both are doing the right thing. It's just a matter of the preferable error message, and the 2nd seems better.

I don't have mind. For now (JDK 19 and 20), the case where it will only likely arise is -agentlib:jdwp --enable-preview on ports that don't have VM continuations. In that case, GetEnv will fail. If virtual threads become permanent in JDK 21 and JVMTI is not updated to work with the alternative implementation of virtual threads then we might have to expand the error message a bit.

-------------

PR: https://git.openjdk.org/jdk/pull/11602