On 24/11/2020 9:38 pm, Jorn Vernee wrote:
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.
Glad you mentioned ExceptionMark! Exactly what is the design philosophy
with respect to exceptions from the called Java method in this API? At
present they are simply ignored. If the thread were then to make JNI
calls that could break rules about not being able to call most JNI
methods with a pending exception. If it makes another upcall does that
immediately "abort" with the pending exception? If we have a case where
the thread auto-detaches what happens then? Will it trigger the Thread
uncaughtExceptionHandler?
David
-----
-------------
PR: https://git.openjdk.java.net/jdk/pull/1266
