Add version constants for Java 19, 20, 21

[morristai]

Ref: https://github.com/openjdk/jdk/blob/dc74ea21f104f49c137476142b6f6340fd34af62/src/java.base/share/native/include/jni.h#L1993

[TheMrMilchmann]

Hate to be that guy but is there any chance to get this merged soon? Ideally alongside support for the added function jboolean IsVirtualThread(JNIEnv *env, jobject obj);. (ref)

[rib]

Yeah, I've recently been slowly trying to look at updating the jni-sys crate a bit.

Ideally I think it would make sense to add the version constants at the same time as adding the new functions, but a follow up PR to add IsVirtualThread for 19 would be good otherwise.

I haven't seen what new API was added for 20 and 21 yet.

[TheMrMilchmann]

I haven't seen what new API was added for 20 and 21 yet.

Neither of these versions seems to contain user-facing API changes. I believe the new version constants were added because IsVirtualThread was technically only in preview in 19 and 20.

[rib]

hmm, that's a pretty unclear concept. I do vaguely recall seeing discussions before about IsVirtualThread being in preview and didn't really know what that was supposed to mean practically.

So is there some kind of change with what can be relied on with the behaviour or IsVirtualThread between versions?

We should be very clear what that notion of being in preview meant here.

Maybe IsVirtualThread should only be exposed once it's out of preview perhaps?

Can it be NULL while it's in preview perhaps?

It would be great if you'd be able to dig up some information about what the preview status changes mean here.

[TheMrMilchmann]

Can it be NULL while it's in preview perhaps?

Preview features are incorporated into the JLS & JVMS. All spec-compliant implementations must implement support for preview features. Thus, this function should always exist.

So is there some kind of change with what can be relied on with the behaviour or IsVirtualThread between versions?

Indeed, the major difference between "stable" and preview features is that the behavior (or the even existence of the API going forward) is not guaranteed. Albeit unlikely, it is possible that a preview function is added but its signature is changed or it is removed in the next release. This is probably something that needs to be considered in the context of #25.

Maybe IsVirtualThread should only be exposed once it's out of preview perhaps?

Ideally no, because the point of preview features is that people can test them and give feedback before the feature is stabilized. As this project is just a low-level wrapper for JNI, I think we should expose preview functions just like everything else.

For further information on preview features, see

• https://openjdk.org/jeps/12
• https://openjdk.org/jeps/8300604

[rib]

Albeit unlikely, it is possible that a preview function is added but its signature is changed or it is removed in the next release

Ooof, yeah that really breaks assumptions with the current way we just have one large vtable that we assume we can safely just append to - we wouldn't be able to handle a situation where an API ended up with a different signature or got removed unless we moved to a union as suggested in #25

Since I was recently scrutinizing the macros in the jni-rs crate I actually realized the jni-rs crate currently incorrectly dereferences v1.2 functions without knowing that the version is >= 1.2 which did generally convince me that it would be a lot better for this crate to expose the function pointers via union instead of one single table.

It is very easy atm to unwittingly use an API that might not be valid for the current known JNI version.

Although I hope there's no example already of an API changing between versions a union would allow for that kind of change.

The main issue probably is just the amount of busy work / boilerplate involved in introducing a union with separate tables for each version. It should probably be done via some kind of build.rs codegen or macros.

Maybe IsVirtualThread should only be exposed once it's out of preview perhaps?

Ideally no, because the point of preview features is that people can test them and give feedback before the feature is stabilized. As this project is just a low-level wrapper for JNI, I think we should expose preview functions just like everything else.

yeah, makes sense.

[rib]

For reference, I'll look at adding IsVirtualThread as a separate follow up.