JDK-8258032 : Reconsider LEAF entry restrictions
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 17
  • Priority: P4
  • Status: Closed
  • Resolution: Won't Fix
  • Submitted: 2020-12-10
  • Updated: 2021-01-13
  • Resolved: 2021-01-13
Related Reports
Duplicate :  
JDK-8257992 - Add a comment to ResetNoHandleMark to explain its usage
Relates :  
JDK-8259373 - c1 and jvmci runtime code use ResetNoHandleMark incorrectly
Relates :  
JDK-8257992 - Add a comment to ResetNoHandleMark to explain its usage
Relates :  
JDK-8259375 - JvmtiExport::jni_Get/SetField_probe calls do not need ResetNoHandleMark
Relates :  
JDK-8259374 - Make ThreadInVMfromNative have ResetNoHandleMark
Description
JRT_LEAF has NoHandleMark and NoSafepointVerifier
JNI_LEAF has NoHandleMark
JVM_LEAF has NoHandleMark

It's unclear why NoHandleMark is used at all and why JNI_LEAF and JVM_LEAF don't have NoSafepointVerifier. It might be more reasonable to just have NoSafepointVerifier for all LEAF entries, but *not* NoHandleMark.
Comments
There are a bunch of land mines in this code if we allow Handles inside of JRT_LEAF and you have to be very careful. If you have a JRT_LEAF and create a Handle, the Handle can leak unless you have a HandleMark. Without the NoHandleMark, your code may compile and run happily without you noticing this. With the NoHandleMark, you must then do: ResetNoHandleMark rm HandleMark hm. this still doesn't make sure you add the HandleMark but at least you'd have a clue. Also, as in the case of Runtime1::access_field_patching, the code doesn't use a JRT_LEAF but remains in the thread_in_Java state, then it goes into JRT_ENTRY. The JRT_ENTRY has a HandleMarkCleaner, which will reset any Handles to the nearest HandleMark. That could be the one that is before call_stub in JavaCalls, wiping away any Handles that you've created before that silently, unless you have a HandleMark. The last line in Runtime1::access_field_patching does a stackwalk. With ZGC, the new StackWaterMark code must have a ResetNoHandleMark and a HandleMark (which it does) or else it will crash. Removing JRT_LEAF NoHandleMark restrictions doesn't help this code. I don't think this is an onerous restriction. I'm going to close this as WNF.
13-01-2021
[~rpressler] This is out for review, can you review it?
04-01-2021
I have a change that lifts the NoHandleMark restriction for JRT_LEAF, but leaves the NoHandleMark restrictions for JNI/JVM_LEAF. It doesn't add a NoSafepointVerifier to JNI JRT_LEAF has NoSafepointVerifier JNI_LEAF has NoHandleMark JVM_LEAF has NoHandleMark The ThreadInVMfromNative wrapper adds a ResetNoHandleMark so that if a thread in native decides to transition to VM, it resets the NoHandleMark without extra calls. Adding NSV to JNI_LEAF doesn't make sense because it's used to not transition from native but may call functions like env->RegisterNatives() that do transition and then NSV doesn't makes sense.
18-12-2020
For JNI_LEAF, the function in native will not stop for safepoints but GC can run while the native function is running. Any oops in the native function can be replaced racingly. If you allocate Handles in the function where thread_in_native, the GC could be concurrently walking the handle block. Also racy. Maybe a NSV doesn't make sense for JNI_LEAF though, because you can have a JNI_LEAF function that does a ThreadInVMfromNative in that function.
17-12-2020
The thing is that even RegisterMap, needed for stack walks, contains oops in Loom and so allocates a Handle. But that's OK because the Handle is not actually used (i.e. across a safepoint), which NoSafepointVerifier should ensure. If we're disallowing safepoints, what's the point in separately disallowing Handles if their allocation is safe as long as there is no safepoint?
17-12-2020
This is a good suggestion to have all of the LEAF entry points have a NoSafepointVerifier, unless the entry transitions into from Native to VM, it needs to be paused. JNI_LEAF (and its equivalent JVM_LEAF) definitely should have a NoHandleMark, because when the thread is _thread_in_native, it is safepoint safe and Handles will not be processed. oops shouldn't be present in JNI_LEAF functions either and maybe the oop constructor should assert if the thread_state is _thread_in_native. I don't see any reason for JRT_LEAF to have a NoHandleMark and there's a lot of code that works hard at calling ResetNoHandleMark for this case. It makes sense to lift this restriction.
17-12-2020