On Tue, 24 Nov 2020 06:12:55 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> src/hotspot/share/prims/universalUpcallHandler.cpp line 36: >> >>> 34: extern struct JavaVM_ main_vm; >>> 35: >>> 36: JNI_ENTRY_CPP_NOENV(void, >>> ProgrammableUpcallHandler::upcall_helper(jobject rec, address buff)) >> >> I do not like this. I think you have a design flaw here. We should not be >> directly calling member functions like this from other native code! The >> entry point should be a simple C-linkage function that is a normal JNI_ENTRY >> (assuming it actually should be a JNI_ENTRY and not JVM_ENTRY?) and that can >> then call the true target. > > Looking closer at this and the potential call-chain I don't think this is > really either a JNI_ENTRY nor a JVM_ENTRY as we would normally define them. > The upcall_stub seems to allow a thread to "tunnel" its way into the VM for a > Java call, rather than going through the normal exported interfaces (i.e. > JNI). I suspect that earlier in the review process it was requested that this > be made some kind of ENTRY but I'm really not sure that fits at all. It may > be cleaner, simpler and clearer to just declare a non-exported method that > use `ThreadInVMFromNative` directly. Ok, that seems like a good idea to me as well. Since this code is different from normal JNI/JVM entries, I'd rather add things like exception marks or vm entry traces as needed manually, than relying on the existing macros, so that the need for each item can be re-evaluated and explained in this context. ------------- PR: https://git.openjdk.java.net/jdk/pull/1266
