RFR: 8256486: Linux/Windows-x86 builds broken after JDK-8254231 [v4]

[Jorn Vernee]

The main assumption is that calling code is fully foreign, and that it 
does not know how to Java handle exceptions at all.

So, if an exception is thrown at this point, we should probably just 
crash, and make it the callee's responsibility to handle exception if 
they need/want to.

Looking at this now, I think I'll switch the upcall itself to use CATCH, 
and then see what we can do on the Java side to crash in a more friendly 
way if there is an active exception when we return from the upcall (such 
as invoking the default uncaught exception handler).

How does that sound?

Jorn

On 24/11/2020 13:35, David Holmes wrote:
> On 24/11/2020 9:38 pm, Jorn Vernee wrote:
>> On Tue, 24 Nov 2020 06:12:55 GMT, David Holmes <dholmes at 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
>>